Re: Expand applicability of aggregate's sortop optimization - Mailing list pgsql-hackers

From Dagfinn Ilmari Mannsåker
Subject Re: Expand applicability of aggregate's sortop optimization
Date
Msg-id 87zft0h8nk.fsf@wibble.ilmari.org
Whole thread Raw
In response to Expand applicability of aggregate's sortop optimization  (Matthias van de Meent <boekewurm+postgres@gmail.com>)
List pgsql-hackers
Matthias van de Meent <boekewurm+postgres@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



pgsql-hackers by date:

Previous
From: Matthias van de Meent
Date:
Subject: Expand applicability of aggregate's sortop optimization
Next
From: Ranier Vilela
Date:
Subject: Re: CREATE DATABASE with filesystem cloning