From 4ba70c021a02f864f660c09bd89591c4292db691 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 15 Aug 2026 15:08:13 -0400 Subject: [PATCH v4] Rework GRAPH_TABLE aggregate/window/SRF rejection using ParseExprKind. Commit f58567105 disallowed aggregates, window functions, and set-returning functions in a GRAPH_TABLE COLUMNS list by inspecting the parse state after transforming the list. This way is not capable of reporting a parse location for the misplaced construct; worse, the patch missed checking for aggregates etc. in GRAPH_TABLE WHERE. Instead give the COLUMNS list and the graph pattern WHERE clause their own ParseExprKind values and enforce the restriction within the parser's transformation functions check_agglevels_and_constraints(), transformWindowFuncCall(), and check_srf_call_placement(). (This reverts the code changes of f58567105, though we keep the test cases and add some more.) This is more consistent with how the parser implements other misplaced-construct checks, and it allows delivery of better error messages. The parsing check rejects only aggregates having level zero. This is intentional: an outer-level aggregate is effectively a constant within the subquery containing GRAPH_TABLE, so there's no reason not to allow it. The case did not work before, but that seems to be only because replace_property_refs_mutator neglected its duty to mutate agglevelsup in the same way as varlevelsup. Note that this changes the SQLSTATE for a rejected aggregate from ERRCODE_FEATURE_NOT_SUPPORTED to ERRCODE_GROUPING_ERROR, consistent with how misplaced aggregates are rejected in other cases. Author: Sami Imseih Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAA5RZ0tvdYODLQvYwVzAxUPe5=E3vSe8Zy7TvrQq+syvGKpHSQ@mail.gmail.com Backpatch-through: 19 --- src/backend/parser/parse_agg.c | 10 +++++ src/backend/parser/parse_clause.c | 32 +-------------- src/backend/parser/parse_expr.c | 8 ++++ src/backend/parser/parse_func.c | 4 ++ src/backend/parser/parse_graphtable.c | 6 +-- src/backend/rewrite/rewriteGraphTable.c | 34 ++++++++++++++++ src/include/parser/parse_node.h | 2 + src/test/regress/expected/graph_table.out | 49 +++++++++++++++++++++-- src/test/regress/sql/graph_table.sql | 9 ++++- 9 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 754a20507d0..a80d11215e2 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -600,6 +600,12 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) break; + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + case EXPR_KIND_GRAPH_TABLE_WHERE: + errkind = true; + + break; + /* * There is intentionally no default: case here, so that the * compiler will warn if we add a new ParseExprKind without @@ -1045,6 +1051,10 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc, case EXPR_KIND_FOR_PORTION: err = _("window functions are not allowed in FOR PORTION OF expressions"); break; + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + case EXPR_KIND_GRAPH_TABLE_WHERE: + errkind = true; + break; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 68b525ffdcc..b1aa13eeb1d 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -946,9 +946,6 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt) ListCell *lc; int resno = 0; bool saved_hasSublinks; - bool saved_hasAggs; - bool saved_hasWindowFuncs; - bool saved_hasTargetSRFs; rel = parserOpenPropGraph(pstate, rgt->graph_name, AccessShareLock); @@ -970,13 +967,6 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt) saved_hasSublinks = pstate->p_hasSubLinks; pstate->p_hasSubLinks = false; - saved_hasAggs = pstate->p_hasAggs; - pstate->p_hasAggs = false; - saved_hasWindowFuncs = pstate->p_hasWindowFuncs; - pstate->p_hasWindowFuncs = false; - saved_hasTargetSRFs = pstate->p_hasTargetSRFs; - pstate->p_hasTargetSRFs = false; - gp = transformGraphPattern(pstate, rgt->graph_pattern); /* @@ -991,7 +981,7 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt) TargetEntry *te; char *colname; - colexpr = transformExpr(pstate, rt->val, EXPR_KIND_SELECT_TARGET); + colexpr = transformExpr(pstate, rt->val, EXPR_KIND_GRAPH_TABLE_COLUMNS); if (rt->name) colname = rt->name; @@ -1041,26 +1031,6 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt) errmsg("subqueries within GRAPH_TABLE reference are not supported"))); pstate->p_hasSubLinks = saved_hasSublinks; - /* - * GRAPH_TABLE cannot yet evaluate aggregate, window, or set-returning - * functions in its COLUMNS list, so prohibit them for now. - */ - if (pstate->p_hasAggs) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("aggregate functions in GRAPH_TABLE COLUMNS are not supported")); - if (pstate->p_hasWindowFuncs) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("window functions in GRAPH_TABLE COLUMNS are not supported")); - if (pstate->p_hasTargetSRFs) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-returning functions in GRAPH_TABLE COLUMNS are not supported")); - pstate->p_hasAggs = saved_hasAggs; - pstate->p_hasWindowFuncs = saved_hasWindowFuncs; - pstate->p_hasTargetSRFs = saved_hasTargetSRFs; - return addRangeTableEntryForGraphTable(pstate, graphid, castNode(GraphPattern, gp), columns, colnames, rgt->alias, false, true); } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 30c889f505f..f1c6aa236cf 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -579,6 +579,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) case EXPR_KIND_GENERATED_COLUMN: case EXPR_KIND_CYCLE_MARK: case EXPR_KIND_PROPGRAPH_PROPERTY: + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + case EXPR_KIND_GRAPH_TABLE_WHERE: /* okay */ break; @@ -1843,6 +1845,8 @@ transformSubLink(ParseState *pstate, SubLink *sublink) case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: case EXPR_KIND_CYCLE_MARK: + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + case EXPR_KIND_GRAPH_TABLE_WHERE: /* okay */ break; case EXPR_KIND_CHECK_CONSTRAINT: @@ -3255,6 +3259,10 @@ ParseExprKindName(ParseExprKind exprKind) return "property definition expression"; case EXPR_KIND_FOR_PORTION: return "FOR PORTION OF"; + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + return "GRAPH_TABLE COLUMNS"; + case EXPR_KIND_GRAPH_TABLE_WHERE: + return "GRAPH_TABLE WHERE"; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index c87804f5d41..0c72b95e4a5 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2848,6 +2848,10 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location) case EXPR_KIND_FOR_PORTION: err = _("set-returning functions are not allowed in FOR PORTION OF expressions"); break; + case EXPR_KIND_GRAPH_TABLE_COLUMNS: + case EXPR_KIND_GRAPH_TABLE_WHERE: + errkind = true; + break; /* * There is intentionally no default: case here, so that the diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c index 73fbfb541f7..e323376f0ea 100644 --- a/src/backend/parser/parse_graphtable.c +++ b/src/backend/parser/parse_graphtable.c @@ -92,7 +92,7 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) if (IsA(field1, A_Star) || IsA(field2, A_Star)) { - if (pstate->p_expr_kind == EXPR_KIND_SELECT_TARGET) + if (pstate->p_expr_kind == EXPR_KIND_GRAPH_TABLE_COLUMNS) ereport(ERROR, errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("\"*\" is not supported here"), @@ -251,7 +251,7 @@ transformGraphElementPattern(ParseState *pstate, GraphElementPattern *gep) gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr); - gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_WHERE); + gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_GRAPH_TABLE_WHERE); /* * Assign collations here for the reason mentioned in the prologue of @@ -387,7 +387,7 @@ transformGraphPattern(ParseState *pstate, GraphPattern *graph_pattern) transformPathPatternList(pstate, graph_pattern->path_pattern_list)); graph_pattern->path_pattern_list = path_pattern_list; - graph_pattern->whereClause = transformExpr(pstate, graph_pattern->whereClause, EXPR_KIND_WHERE); + graph_pattern->whereClause = transformExpr(pstate, graph_pattern->whereClause, EXPR_KIND_GRAPH_TABLE_WHERE); assign_expr_collations(pstate, graph_pattern->whereClause); return (Node *) graph_pattern; diff --git a/src/backend/rewrite/rewriteGraphTable.c b/src/backend/rewrite/rewriteGraphTable.c index 0eaf28b3de5..9b5495ffaa2 100644 --- a/src/backend/rewrite/rewriteGraphTable.c +++ b/src/backend/rewrite/rewriteGraphTable.c @@ -1039,6 +1039,40 @@ replace_property_refs_mutator(Node *node, struct replace_property_refs_context * return (Node *) newvar; } + else if (IsA(node, Aggref)) + { + Aggref *aggref; + + /* Copy the Aggref node and mutate its sub-structure */ + aggref = (Aggref *) expression_tree_mutator(node, + replace_property_refs_mutator, + context); + + /* + * An aggregate is allowed in a graph table expression, but only if + * it's an outer aggregate. Since it will be in a subquery after the + * rewrite, we have to increase the level by one. + */ + Assert(aggref->agglevelsup > 0); + aggref->agglevelsup++; + + return (Node *) aggref; + } + else if (IsA(node, GroupingFunc)) + { + GroupingFunc *grp; + + /* Copy the GroupingFunc node and mutate its sub-structure */ + grp = (GroupingFunc *) expression_tree_mutator(node, + replace_property_refs_mutator, + context); + + /* Like Aggref, this should be an outer-level reference */ + Assert(grp->agglevelsup > 0); + grp->agglevelsup++; + + return (Node *) grp; + } else if (IsA(node, GraphPropertyRef)) { GraphPropertyRef *gpr = (GraphPropertyRef *) node; diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index f7f4ba6c2a8..2b8ac813554 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -84,6 +84,8 @@ typedef enum ParseExprKind EXPR_KIND_GENERATED_COLUMN, /* generation expression for a column */ EXPR_KIND_CYCLE_MARK, /* cycle mark value */ EXPR_KIND_PROPGRAPH_PROPERTY, /* derived property expression */ + EXPR_KIND_GRAPH_TABLE_COLUMNS, /* GRAPH_TABLE COLUMNS list item */ + EXPR_KIND_GRAPH_TABLE_WHERE, /* WHERE in a GRAPH_TABLE pattern */ } ParseExprKind; diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index cde3114ebf4..04b58a625ad 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -471,13 +471,54 @@ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.* IS NOT NULL)-[ ERROR: "*" not allowed here LINE 1: ...M GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.* IS NOT... ^ --- aggregate, window, and set-returning functions are not supported in COLUMNS +-- aggregate, grouping, window, and set-returning functions are not allowed +-- in the COLUMNS list or the graph pattern WHERE, except for outer-level aggs SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (count(*) AS num)); -ERROR: aggregate functions in GRAPH_TABLE COLUMNS are not supported +ERROR: aggregate functions are not allowed in GRAPH_TABLE COLUMNS +LINE 1: ...APH_TABLE (myshop MATCH (c IS customers) COLUMNS (count(*) A... + ^ +SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (GROUPING(c.customer_id) AS g)); +ERROR: grouping operations are not allowed in GRAPH_TABLE COLUMNS +LINE 1: ...APH_TABLE (myshop MATCH (c IS customers) COLUMNS (GROUPING(c... + ^ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (row_number() OVER () AS rn)); -ERROR: window functions in GRAPH_TABLE COLUMNS are not supported +ERROR: window functions are not allowed in GRAPH_TABLE COLUMNS +LINE 1: ...APH_TABLE (myshop MATCH (c IS customers) COLUMNS (row_number... + ^ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (generate_series(1, 2) AS gs)); -ERROR: set-returning functions in GRAPH_TABLE COLUMNS are not supported +ERROR: set-returning functions are not allowed in GRAPH_TABLE COLUMNS +LINE 1: ...APH_TABLE (myshop MATCH (c IS customers) COLUMNS (generate_s... + ^ +SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE count(c.customer_id) > 0) COLUMNS (c.name AS nm)); +ERROR: aggregate functions are not allowed in GRAPH_TABLE WHERE +LINE 1: ...M GRAPH_TABLE (myshop MATCH (c IS customers WHERE count(c.cu... + ^ +SELECT EXISTS(SELECT num FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (count(o.customer_id) AS num)) t) FROM customers o; + exists +-------- + t +(1 row) + +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers) WHERE count(o.customer_id) > 0 COLUMNS (c.name AS nm)) t) FROM customers o; + exists +-------- + t +(1 row) + +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE count(o.customer_id) > 0) COLUMNS (c.name AS nm)) t) FROM customers o; + exists +-------- + t +(1 row) + +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE GROUPING(o.customer_id) = 1) COLUMNS (c.name AS nm)) t) FROM customers o GROUP BY customer_id; + exists +-------- + f + f + f +(3 rows) + -- consecutive element patterns with same kind SELECT * FROM GRAPH_TABLE (g1 MATCH ()() COLUMNS (1 as one)); ERROR: adjacent vertex patterns are not supported diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 7a4189833d8..8a1173aba92 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -306,10 +306,17 @@ SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS el1 | vl1)-[conn]->(dest) COLUMNS (c SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (c.*)); -- star anywhere else is not allowed as a property reference SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.* IS NOT NULL)-[IS customer_orders]->(o IS orders) COLUMNS (c.name)); --- aggregate, window, and set-returning functions are not supported in COLUMNS +-- aggregate, grouping, window, and set-returning functions are not allowed +-- in the COLUMNS list or the graph pattern WHERE, except for outer-level aggs SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (count(*) AS num)); +SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (GROUPING(c.customer_id) AS g)); SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (row_number() OVER () AS rn)); SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (generate_series(1, 2) AS gs)); +SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE count(c.customer_id) > 0) COLUMNS (c.name AS nm)); +SELECT EXISTS(SELECT num FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (count(o.customer_id) AS num)) t) FROM customers o; +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers) WHERE count(o.customer_id) > 0 COLUMNS (c.name AS nm)) t) FROM customers o; +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE count(o.customer_id) > 0) COLUMNS (c.name AS nm)) t) FROM customers o; +SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE GROUPING(o.customer_id) = 1) COLUMNS (c.name AS nm)) t) FROM customers o GROUP BY customer_id; -- consecutive element patterns with same kind SELECT * FROM GRAPH_TABLE (g1 MATCH ()() COLUMNS (1 as one)); SELECT * FROM GRAPH_TABLE (g1 MATCH -> COLUMNS (1 AS one)); -- 2.52.0