Re: [PATCH] GROUP BY ALL

From: David Christensen <david(at)pgguru(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
Subject: Re: [PATCH] GROUP BY ALL
Date: 2025-09-26 16:17:29
Message-ID: CAHM0NXiL+KDNygg5xed-cuQmMQvwwtUoZ8YQstL0Tt7kEZ9g3w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 26, 2025 at 11:13 AM David Christensen <david(at)pgguru(dot)net> wrote:
>
> On Fri, Sep 26, 2025 at 11:05 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > David Christensen <david(at)pgguru(dot)net> writes:
> > > Here is a rebased version with a few more tests. I also changed the
> > > main check here to using `!contain_agg_clause` instead of
> > > `!IsA(Aggref))` directly. (This was defined in `optimizer/clauses.h`,
> > > but we already are pulling in `optimizer.h`, so it felt valid to me.)
> >
> > contain_agg_clause will blow up on a SubLink, so I doubt this is
> > gonna be robust.
>
> Fair enough, see that Assert now; easy enough to make a new
> expression_tree_walker that only looks for Aggref and short-circuits
> SubLink (which I assume is the right behavior here, but might have to
> add some more tests/play around with subqueries in the GROUP BY ALL
> part).

Or contain_aggs_of_level(), assuming I can suss out how to get the
current level... :D Anyway, will mess with this for a bit.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabrice Chapuis 2025-09-26 16:21:58 Re: Issue with logical replication slot during switchover
Previous Message David Christensen 2025-09-26 16:13:04 Re: [PATCH] GROUP BY ALL