Matthias van de Meent <boekewurm+postg...@gmail.com> writes:

> PFA the small patch that implements this.

I don't have enough knowledge to have an opinion on most of the patch
other than it looks okay at a glance, but the list API usage could be
updated to more modern variants:

> diff --git a/src/backend/optimizer/plan/planagg.c 
> b/src/backend/optimizer/plan/planagg.c
> index afb5445b77..d8479fe286 100644
> --- a/src/backend/optimizer/plan/planagg.c
> +++ b/src/backend/optimizer/plan/planagg.c
> @@ -253,6 +253,16 @@ can_minmax_aggs(PlannerInfo *root, List **context)
>               if (list_length(aggref->args) != 1)
>                       return false;           /* it couldn't be MIN/MAX */
>  
> +             /*
> +              * We might implement the optimization when a FILTER clause is 
> present
> +              * by adding the filter to the quals of the generated subquery. 
>  For
> +              * now, just punt.
> +              */
> +             if (aggref->aggfilter != NULL)
> +                     return false;
> +
> +             curTarget = (TargetEntry *) linitial(aggref->args);

This could be linitial_node(TargetEntry, aggref->args).

> +                     if (list_length(aggref->aggorder) > 1)
> +                             return false;
> +
> +                     orderClause = castNode(SortGroupClause, 
> linitial(aggref->aggorder));

This could be linitial_node(SortGroupClause, aggref->aggorder).

> +                     if (orderClause->sortop != aggsortop)
> +                     {
> +                             List   *btclasses;
> +                             ListCell *cell;
> +                             bool    match = false;
> +
> +                             btclasses = 
> get_op_btree_interpretation(orderClause->sortop);
> +
> +                             foreach(cell, btclasses)
> +                             {
> +                                     OpBtreeInterpretation *interpretation;
> +                                     interpretation = (OpBtreeInterpretation 
> *) lfirst(cell);

This could be foreach_ptr(OpBtreeInterpretation, interpretation, btclasses),
which also eliminates the need for the explicit `ListCell *` variable
and lfirst() call.

- ilmari


Reply via email to