Re: GRAPH_TABLE: aggregates/window/set-returning functions in COLUMNS crash the backend

On Tue, Jun 16, 2026 at 3:57 PM Ewan Young <kdbase(dot)hack(at)gmail(dot)com> wrote:
>
> 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.

We might need minor changes to the comment, but this part looks good to me.

>
> 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

The problem with aggregates and window functions is that there is not
enough context for performing aggregation in COLUMNs. Set returning
functions are different, they can be safely evaluated in queries that
replace the GRAPH_TABLE construct. Looking at the standard graph table
columns clause is a list of graph table column definitions, each of
which is a <value expression> which can be <collection value
expression>. So it looks like the standard doesn't prohibit SRFs in
COLUMNs clause and we are evaluating them correctly. However, I am
wondering whether GRAPH_TABLE is expected to output only one row for
every matching walk/substructure from the graph; there are GRAPH_TABLE
shapes that seem to suggest one row per matching pattern. SRFs violate
that rule. Maybe that's why they should be prohibited in COLUMNs. But
I don't think I have understood it well. Peter, can you please clarify
whether SRFs can be part of COLUMNs clause or not?

--
Best Wishes,
Ashutosh Bapat