From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Christensen <david@pgguru.net>
cc: Andrey Borodin <x4mmm@yandex-team.ru>,
        Peter Eisentraut <peter@eisentraut.org>,
        pgsql-hackers <pgsql-hackers@postgresql.org>,
        "David G. Johnston" <david.g.johnston@gmail.com>,
        Jelte Fennema-Nio <postgres@jeltef.nl>
Subject: Re: [PATCH] GROUP BY ALL
In-reply-to: 
 <CAHM0NXgPhe_OF81MyA9ocWZ=b+oEAHtQHoDXw4DverPdbkXzpg@mail.gmail.com>
References: <4D2047B0-E8D8-472B-B7E8-61206B1E6AFA@yandex-team.ru>
 <F694DABC-6A42-4614-B254-643180B9D6F6@pgguru.net>
 <CAHM0NXj1Gv_96WxZkiLKhxJVaYOGcmdZ2hApNcvWQ5HxV84Q=A@mail.gmail.com>
 <4083063.1758902694@sss.pgh.pa.us>
 <CAHM0NXgPhe_OF81MyA9ocWZ=b+oEAHtQHoDXw4DverPdbkXzpg@mail.gmail.com>
Comments: In-reply-to David Christensen <david@pgguru.net>
	message dated "Fri, 26 Sep 2025 11:13:04 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <4085063.1758903815.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Fri, 26 Sep 2025 12:23:35 -0400
Message-ID: <4085064.1758903815@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/4085064.1758903815%40sss.pgh.pa.us>
Precedence: bulk

David Christensen <david@pgguru.net> writes:
> On Fri, Sep 26, 2025 at 11:05 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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).

No, I think the correct behavior would have to be to descend into
SubLinks to see if they contain any aggregates belonging to the
outer query level.

However (looks around) we do already have that code.
See contain_aggs_of_level.  (contain_agg_clause is essentially
a simplified version that is okay to use in the planner because
it's already gotten rid of sublinks.)

What mainly concerns me at this point is whether we've identified
aggregate levels at the point in parsing where you want to run this.
I have a bit of a worry that that might interact with grouping.
Presumably the SQL committee thought about that, so it's probably
soluble, but ...

			regards, tom lane