Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Ewan Young <kdbase(dot)hack(at)gmail(dot)com>
Subject: Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE
Date: 2026-08-15 19:21:58
Message-ID: 207328.1786821718@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Sami Imseih <samimseih(at)gmail(dot)com> writes:
> v3 attached. No .c changes from v2.

I looked this over. The general pattern of adding more EXPR_KIND
values looks fine, but I really didn't like the nonstandard way in
which you tested for GRAPH_TABLE in check_agglevels_and_constraints.
Testing before running up to the aggregate's semantic level is just
wrong: it would fail to reject an aggregate within a subquery within
one of these GRAPH_TABLE clauses. And if we're testing at some other
level than the semantic level, should we also reject GRAPH_TABLE
context at intermediate parse levels?

This was ugly enough that it motivated me to go look at exactly why
an outer aggregate doesn't work here, in hopes of removing the
inconsistent restriction. I found it: replace_property_refs_mutator,
which increments Vars' varlevelsup to account for the fact that
they're being pushed into a subquery, failed to do the equivalent
thing for Aggrefs and GroupingFuncs. (Compare, for instance,
IncrementVarSublevelsUp.) The case seems to Just Work after fixing
that, so I changed check_agglevels_and_constraints to enforce the
restriction in just the same way as it does for other clauses where
we disallow aggs.

As a minor improvement, you can just use the "errkind = true" option
in these error-reporting functions; less code, fewer strings for the
translators to deal with, same or better message wording.

v4 attached. I think this is committable if you don't see anything
else to change.

regards, tom lane

Attachment Content-Type Size
v4-0001-Rework-GRAPH_TABLE-aggregate-window-SRF-rejection.patch text/x-diff 16.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2026-08-15 20:02:35 Re: GIN VACUUM can corrupt internal posting tree pages
Previous Message Andrey Borodin 2026-08-15 17:45:32 Re: GIN VACUUM can corrupt internal posting tree pages