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.fnst@cn.fujitsu.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