Re: Expand applicability of aggregate's sortop optimization

From: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Expand applicability of aggregate's sortop optimization
Date: 2024-05-08 10:58:55
Message-ID: 87zft0h8nk.fsf@wibble.ilmari.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Matthias van de Meent <boekewurm+postgres(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2024-05-08 11:16:18 Re: CREATE DATABASE with filesystem cloning
Previous Message Matthias van de Meent 2024-05-08 10:13:29 Expand applicability of aggregate's sortop optimization