| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: GRAPH_TABLE: aggregates/window/set-returning functions in COLUMNS crash the backend |
| Date: | 2026-06-16 10:27:19 |
| Message-ID: | CAON2xHN_Mqjv1uLWsxMu1kE=At8a7iZNrouixAurG91LX_q39Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Ashutosh,
On Tue, Jun 16, 2026 at 5:51 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Tue, Jun 16, 2026 at 2:41 PM Ewan Young <kdbase(dot)hack(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > While testing SQL/PGQ I found that putting an aggregate, window
> > function, or set-returning function in the COLUMNS list of a
> > GRAPH_TABLE query crashes the backend.
> >
> > Minimal reproducer:
> >
> > CREATE TABLE v (id int PRIMARY KEY);
> > INSERT INTO v VALUES (1);
> > CREATE PROPERTY GRAPH g VERTEX TABLES (v);
> >
> > SELECT max(c) FROM GRAPH_TABLE (g MATCH (x IS v) COLUMNS (count(*) AS c));
> > On an assert-enabled build this trips
> >
> > TRAP: failed Assert("econtext->ecxt_aggvalues != NULL"),
> > File: "execExprInterp.c", Line: 1969
> > and on a non-assert build it fails at execution with
> >
> > ERROR: Aggref found in non-Agg plan node
> > A window function in COLUMNS behaves the same way; a set-returning
> > function is silently accepted and produces nonsensical results.
> >
> > Root cause: transformRangeGraphTable() (parser/parse_clause.c)
> > transforms the COLUMNS expressions with EXPR_KIND_SELECT_TARGET, which
> > permits aggregates, window functions and SRFs. GRAPH_TABLE has no
> > machinery to evaluate them, though: rewriteGraphTable.c copies the
> > COLUMNS target list verbatim into a freshly built subquery whose
> > hasAggs/hasWindowFuncs flags are never set, so the planner builds no
> > Agg/WindowAgg node and the Aggref/WindowFunc reaches the executor.
> > (As a side effect p_hasAggs also leaks into the enclosing query,
> > yielding spurious "must appear in the GROUP BY clause" errors for some
> > other COLUMNS shapes.)
> >
> > These constructs are not meaningful in a GRAPH_TABLE COLUMNS list, so
> > the attached patch rejects them at parse-analysis time, the same way
> > every other non-aggregating context does. It adds a dedicated
> > EXPR_KIND_GRAPH_TABLE_COLUMNS and wires it into the aggregate, window
> > and set-returning-function checks, producing errors such as
> >
> > ERROR: aggregate functions are not allowed in GRAPH_TABLE COLUMNS
> > Plain column references and subqueries are unaffected (subqueries
> > continue to be rejected as before). A regression test is added to
> > graph_table.sql, and "make check" passes.
>
> Thanks for the report and the patch.
>
> Right now we do not support quantified element patterns like
> (a)->{1-5}, but when we do that it's allowed to have aggregates in
> COLUMNs e.g. count(a) or sum(a.val). So the patch is not going in the
> right direction. Instead we should check treat pstate->p_hasAggs and
> pstate->p_hasWindowFuncs just like pstate->hasSublinks and throw an
> error in transformRangeGraphTable() for now. When we will support
> quantified element patterns, we will need to check the arguments of
> the aggregates. If the arguments are property references with higher
> degree, we will allow the aggregates, otherwise not.
Makes sense, the dedicated EXPR_KIND was overkill -- and you're right
that we'll want to allow aggregates over higher-degree property
references once quantified element patterns land, so a blanket
rejection keyed off the expression kind would be in the way.
v2 attached. It drops EXPR_KIND_GRAPH_TABLE_COLUMNS entirely and
instead follows the existing p_hasSubLinks pattern in
transformRangeGraphTable(): save and clear p_hasAggs / p_hasWindowFuncs
around the COLUMNS transformation, then throw a "not supported" error
if either got set. There's a comment noting the aggregate restriction
is temporary and will need to check the aggregate arguments (degree of
the property refs) rather than reject everything once quantified
patterns are supported.
I also clear and check p_hasTargetSRFs the same way, since
set-returning functions hit the same class of failure -- the rewriter
doesn't set hasTargetSRFs on the generated subquery either, so an SRF
in COLUMNS reaches the executor unevaluated. Happy to drop that hunk if
you'd rather keep this patch scoped to aggregates and window functions
>
> --
> Best Wishes,
> Ashutosh Bapat
Regards,
Ewan Young
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Disallow-aggregates-window-functions-and-SRFs-in-GR.patch | application/octet-stream | 6.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zhijie Hou (Fujitsu) | 2026-06-16 10:32:14 | RE: Fix race in ReplicationSlotRelease for ephemeral slots |
| Previous Message | Dmitry Dolgov | 2026-06-16 10:13:47 | Re: Randomize B-Tree page split location to avoid oscillating patterns |