Re: Use list_delete_xxxcell O(1) instead of list_delete_ptr O(N) in some places

On Fri, 16 Oct 2020 at 16:42, Hou, Zhijie <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com> wrote:
> And after checking the code again and I found two more places which can be improved.
>
> 1.
> --- a/src/backend/parser/parse_expr.c
> +++ b/src/backend/parser/parse_expr.c
> @@ -1702,7 +1702,7 @@ transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref)
> */
> if (maref->colno == maref->ncolumns)
> pstate->p_multiassign_exprs =
> - list_delete_ptr(pstate->p_multiassign_exprs, tle);
> + list_delete_last(pstate->p_multiassign_exprs);
>
> Based on the logic above in function transformMultiAssignRef,
> I found 'tle' is always the last one in list ' pstate->p_multiassign_exprs ' ,
> So list_delete_last seems can be used here.

Yeah. After a bit of looking I agree. There's a similar assumption
there already with:

/*
* Second or later column in a multiassignment. Re-fetch the
* transformed SubLink or RowExpr, which we assume is still the last
* entry in p_multiassign_exprs.
*/
Assert(pstate->p_multiassign_exprs != NIL);
tle = (TargetEntry *) llast(pstate->p_multiassign_exprs);

> 2.
>
> + nameEl_idx = foreach_current_index(option);
> }
> }
>
> @@ -405,7 +407,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
> }
> sname = rv->relname;
> /* Remove the SEQUENCE NAME item from seqoptions */
> - seqoptions = list_delete_ptr(seqoptions, nameEl);
> + seqoptions = list_delete_nth_cell(seqoptions, nameEl_idx);
>
> Add a new var ' nameEl_idx ' to catch the index.

Yeah. That looks fine too.

Pushed.

David