On Fri, Mar 27, 2026 at 3:28 PM Peter Eisentraut <[email protected]> wrote: > > On 26.03.26 08:08, Ashutosh Bapat wrote: > > Consider following query > > Query1 > > SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS > > customers)-[IS customer_orders]->(o IS orders WHERE o.order_id = x1.a) > > COLUMNS (x1.name AS customer_name, x1.customer_id AS cid)) g; > > > > x1 is a table referenced in from clause as well as an element pattern > > variable in path pattern. If we don't allow backward (cross) > > referencing, x1.a in the element pattern o resolves to the table x1's > > column a, but x1.name resolves to element x1's property reference > > name. So within the same graph pattern x1 may get resolved to two > > different things, even though x1 was declared before using it. Is that > > how we expect it to happen? Let's call this inconsistency1. > > I don't think this interpretation is correct. > > Even if we don't support non-local element variable references, we still > need to *recognize* them. We just don't need to be able to process > them, we can error out if we see them.
patch 0002 in the attached patchset implements it this way. It collects all the element variables before transforming the path pattern. Later it uses this list to detect non-local variable references and throws error. While implementing this I found another bug. In trasformColumnRef() we try to resolve the column ref as a table.column first which may resolve a property reference if a lateral table has the same name as the variable name and has a column with the same name as a property. Given that variables in the GRAPH_TABLE form the innermost namespace, we should try resolving the column reference as a property before lateral table reference. Since we do not allow subquery inside GRAPH_TABLE, there is no way a tables/columns/function will form the innermost namespace. The patch has a test demonstrating the bug and also the fix. Please let me know what you do you think of the fix. > > > 0001: is a small adjustment to make sure that we add an element > > variable name to the graph table namespace only once. This isn't a > > problem right now, but we will have to do that anyway in [1] and it > > has some effect one 0002. > > committed Thanks. > > > 0002: prohibits cross element variable references within graph_table > > clause. It adds a new member GraphTableParseState::cur_variable to > > track the variable name of the current pattern being transformed. We > > can not use llast() simply because a. pattern currently being > > transformed may not have name, so llast will be misleading b. changes > > in 0001. We may want to combine 0001 and 0002 when committing. > > (under discussion) > > > 0003: prohibits consecutive element patterns of the same kind > > committed (I made the error message wording a bit more compact.) That looks better. Thanks. > > > 0004: some cleanup remaining from > > 5282bf535e474dc2517f2e835d147420ae2144de. We may want to wait to > > accumulate more cleanups. > > I don't think this patch goes into the right direction. The existing > code seems preferable. > Removed from my patch set. -- Best Wishes, Ashutosh Bapat
From 83b76cacf96364137431510c6dd581550dd8bc36 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Fri, 27 Mar 2026 17:57:33 +0530 Subject: [PATCH v20260327 1/2] Property references are preferred over regular column references When a ColumnRef can be resolved as a graph table property reference and a lateral table column reference prefer the graph table property reference since element pattern variables in the GRAPH_TABLE clause form the innermost namespace. Author: Ashutosh Bapat <[email protected]> --- src/backend/parser/parse_expr.c | 14 ++++++++++---- src/test/regress/expected/graph_table.out | 18 ++++++++++++++---- src/test/regress/sql/graph_table.sql | 6 +++++- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 312dfdc182a..d5fc9e7d6bc 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -614,6 +614,16 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) return node; } + /* + * Element pattern variables in a GRAPH_TABLE clause form the innermost + * namespace since we do not allow subqueries in GRAPH_TABLE patterns. Try + * to resolve the column reference as a graph table property reference + * before trying to resolve it as a regular column reference. + */ + node = transformGraphTablePropertyRef(pstate, cref); + if (node != NULL) + return node; + /*---------- * The allowed syntaxes are: * @@ -826,10 +836,6 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) break; } - /* Try it as a graph table property reference. */ - if (node == NULL) - node = transformGraphTablePropertyRef(pstate, cref); - /* * Now give the PostParseColumnRefHook, if any, a chance. We pass the * translation-so-far so that it can throw an error if it wishes in the diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index 2fc5a2e8c5a..01f480d1a57 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -236,14 +236,24 @@ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers)->(o IS orders) COLUMNS (2 rows) -- lateral test -CREATE TABLE x1 (a int, b text); +-- Use table with a column name same as a property in the property graph so as +-- to test resolution preferences. Property references are preferred over +-- lateral table references. +CREATE TABLE x1 (a int, address text); INSERT INTO x1 VALUES (1, 'one'), (2, 'two'); SELECT * FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS customer_orders]->(o IS orders) COLUMNS (c.name AS customer_name, c.customer_id AS cid)); - a | b | customer_name | cid ----+-----+---------------+----- - 1 | one | customer1 | 1 + a | address | customer_name | cid +---+---------+---------------+----- + 1 | one | customer1 | 1 (1 row) +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers WHERE x1.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (x1.name AS customer_name, x1.customer_id AS cid, o.order_id)) g; + a | customer_name | cid | order_id +---+---------------+-----+---------- + 1 | customer1 | 1 | 1 + 2 | customer1 | 1 | 1 +(2 rows) + DROP TABLE x1; CREATE TABLE v1 ( id int PRIMARY KEY, diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 7c2438e84a3..30e450b3842 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -151,9 +151,13 @@ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers)-[IS customer_orders | c SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers)->(o IS orders) COLUMNS (c.name, o.ordered_when)) ORDER BY 1; -- lateral test -CREATE TABLE x1 (a int, b text); +-- Use table with a column name same as a property in the property graph so as +-- to test resolution preferences. Property references are preferred over +-- lateral table references. +CREATE TABLE x1 (a int, address text); INSERT INTO x1 VALUES (1, 'one'), (2, 'two'); SELECT * FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS customer_orders]->(o IS orders) COLUMNS (c.name AS customer_name, c.customer_id AS cid)); +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers WHERE x1.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (x1.name AS customer_name, x1.customer_id AS cid, o.order_id)) g; DROP TABLE x1; CREATE TABLE v1 ( base-commit: 6857947db5bbec823226cd5234f088524f933caa -- 2.34.1
From a71ab4d8182c1693353d6be55afa47b74e41bb6d Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Wed, 25 Mar 2026 17:15:10 +0530 Subject: [PATCH v20260327 2/2] Cross variable references in graph pattern causes segfault When converting the WHERE clause in an element pattern, generate_query_for_graph_path() calls replace_property_refs() to replace the property references in it. Only the current graph element pattern is passed as the context for replacement. If there are references to variables from other element patterns, it causes a segmentation fault (an assertion failure in an Assert enabled build) since it does not find path_element object corresponding to those variables. We do not support forward and backward variable references within a graph table clause. Hence prohibit all the cross references. Author: Ashutosh Bapat <[email protected]> Reported by: Man Zeng <[email protected]> Investigated by: Henson Choi <[email protected]> Reviewed by: Junwang Zhao <[email protected]> --- src/backend/parser/parse_graphtable.c | 43 +++++++++++++++++++++-- src/include/parser/parse_node.h | 3 ++ src/test/regress/expected/graph_table.out | 20 +++++++++++ src/test/regress/sql/graph_table.sql | 8 +++++ 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c index 49ec5c469c2..30ddce5aa9f 100644 --- a/src/backend/parser/parse_graphtable.c +++ b/src/backend/parser/parse_graphtable.c @@ -109,10 +109,25 @@ transformGraphTablePropertyRef(ParseState *pstate, ColumnRef *cref) if (list_member(gpstate->variables, field1)) { - GraphPropertyRef *gpr = makeNode(GraphPropertyRef); + GraphPropertyRef *gpr; HeapTuple pgptup; Form_pg_propgraph_property pgpform; + /* + * If we are transforming expression in an element pattern, + * property references containing only that variable are allowed. + */ + if (gpstate->cur_gep) + { + if (!gpstate->cur_gep->variable || + strcmp(elvarname, gpstate->cur_gep->variable) != 0) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("non-local element variable reference is not supported"), + parser_errposition(pstate, cref->location)); + } + + gpr = makeNode(GraphPropertyRef); pgptup = SearchSysCache2(PROPGRAPHPROPNAME, ObjectIdGetDatum(gpstate->graphid), CStringGetDatum(propname)); if (!HeapTupleIsValid(pgptup)) ereport(ERROR, @@ -230,14 +245,17 @@ transformGraphElementPattern(ParseState *pstate, GraphElementPattern *gep) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("element pattern quantifier is not supported"))); - if (gep->variable) - gpstate->variables = list_append_unique(gpstate->variables, makeString(pstrdup(gep->variable))); + Assert(!gpstate->cur_gep); + + gpstate->cur_gep = gep; gep->labelexpr = transformLabelExpr(gpstate, gep->labelexpr); gep->whereClause = transformExpr(pstate, gep->whereClause, EXPR_KIND_WHERE); assign_expr_collations(pstate, gep->whereClause); + gpstate->cur_gep = NULL; + return (Node *) gep; } @@ -306,6 +324,9 @@ static Node * transformPathPatternList(ParseState *pstate, List *path_pattern) { List *result = NIL; + GraphTableParseState *gpstate = pstate->p_graph_table_pstate; + + Assert(gpstate); /* Grammar doesn't allow empty path pattern list */ Assert(list_length(path_pattern) > 0); @@ -319,6 +340,22 @@ transformPathPatternList(ParseState *pstate, List *path_pattern) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("multiple path patterns in one GRAPH_TABLE clause not supported"))); + /* + * Collect all the variables in the path pattern into the + * GraphTableParseState so that we can detect any non-local element + * variable references. We need to do this before transforming the path + * pattern so as to detect forward references to element variables in the + * WHERE clause of an element pattern. + */ + foreach_node(List, path_term, path_pattern) + { + foreach_node(GraphElementPattern, gep, path_term) + { + if (gep->variable) + gpstate->variables = list_append_unique(gpstate->variables, makeString(pstrdup(gep->variable))); + } + } + foreach_node(List, path_term, path_pattern) result = lappend(result, transformPathTerm(pstate, path_term)); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index fc2cbeb2083..1a7da399f1b 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -110,6 +110,9 @@ typedef struct GraphTableParseState Oid graphid; /* OID of the graph being referenced */ List *variables; /* list of element pattern variables in * GRAPH_TABLE */ + GraphElementPattern *cur_gep; /* The element pattern being transformed. + * NULL if no element pattern is being + * transformed. */ } GraphTableParseState; /* diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index 01f480d1a57..b579e3df635 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -254,6 +254,16 @@ SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers WHERE x1.ad 2 | customer1 | 1 | 1 (2 rows) +-- non-local property references are not allowed, even if a lateral column +-- reference is available +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers)-[IS customer_orders]->(o IS orders WHERE o.order_id = x1.a) COLUMNS (x1.name AS customer_name, x1.customer_id AS cid, o.order_id)) g; -- error +ERROR: non-local element variable reference is not supported +LINE 1: ...customer_orders]->(o IS orders WHERE o.order_id = x1.a) COLU... + ^ +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS customer_orders]->(x1 IS orders) COLUMNS (c.name AS customer_name, c.customer_id AS cid, x1.order_id)) g; -- error +ERROR: non-local element variable reference is not supported +LINE 1: ...tomers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS ... + ^ DROP TABLE x1; CREATE TABLE v1 ( id int PRIMARY KEY, @@ -449,6 +459,16 @@ SELECT * FROM GRAPH_TABLE (g1 MATCH ()-> ->() COLUMNS (1 AS one)); ERROR: edge pattern must be preceded by a vertex pattern LINE 1: SELECT * FROM GRAPH_TABLE (g1 MATCH ()-> ->() COLUMNS (1 AS ... ^ +-- non-local element variable reference with element patterns without variable +-- names +SELECT * FROM GRAPH_TABLE (g1 MATCH (a)-[WHERE a.vprop1 = 10]->(c) COLUMNS (a.vname AS aname, c.vname AS cname)); +ERROR: non-local element variable reference is not supported +LINE 1: SELECT * FROM GRAPH_TABLE (g1 MATCH (a)-[WHERE a.vprop1 = 10... + ^ +SELECT * FROM GRAPH_TABLE (g1 MATCH (WHERE b.eprop1 = 10001)-[b]->(c) COLUMNS (b.ename AS bname, c.vname AS cname)); +ERROR: non-local element variable reference is not supported +LINE 1: SELECT * FROM GRAPH_TABLE (g1 MATCH (WHERE b.eprop1 = 10001)... + ^ -- select all the properties across all the labels associated with a given type -- of graph element SELECT * FROM GRAPH_TABLE (g1 MATCH (src)-[conn]->(dest) COLUMNS (src.vname AS svname, conn.ename AS cename, dest.vname AS dvname, src.vprop1 AS svp1, src.vprop2 AS svp2, src.lprop1 AS slp1, dest.vprop1 AS dvp1, dest.vprop2 AS dvp2, dest.lprop1 AS dlp1, conn.eprop1 AS cep1, conn.lprop2 AS clp2)); diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 30e450b3842..4ff98817420 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -158,6 +158,10 @@ CREATE TABLE x1 (a int, address text); INSERT INTO x1 VALUES (1, 'one'), (2, 'two'); SELECT * FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS customer_orders]->(o IS orders) COLUMNS (c.name AS customer_name, c.customer_id AS cid)); SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers WHERE x1.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (x1.name AS customer_name, x1.customer_id AS cid, o.order_id)) g; +-- non-local property references are not allowed, even if a lateral column +-- reference is available +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (x1 IS customers)-[IS customer_orders]->(o IS orders WHERE o.order_id = x1.a) COLUMNS (x1.name AS customer_name, x1.customer_id AS cid, o.order_id)) g; -- error +SELECT x1.a, g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a)-[IS customer_orders]->(x1 IS orders) COLUMNS (c.name AS customer_name, c.customer_id AS cid, x1.order_id)) g; -- error DROP TABLE x1; CREATE TABLE v1 ( @@ -298,6 +302,10 @@ SELECT * FROM GRAPH_TABLE (g1 MATCH ()() COLUMNS (1 as one)); SELECT * FROM GRAPH_TABLE (g1 MATCH -> COLUMNS (1 AS one)); SELECT * FROM GRAPH_TABLE (g1 MATCH ()-[]- COLUMNS (1 AS one)); SELECT * FROM GRAPH_TABLE (g1 MATCH ()-> ->() COLUMNS (1 AS one)); +-- non-local element variable reference with element patterns without variable +-- names +SELECT * FROM GRAPH_TABLE (g1 MATCH (a)-[WHERE a.vprop1 = 10]->(c) COLUMNS (a.vname AS aname, c.vname AS cname)); +SELECT * FROM GRAPH_TABLE (g1 MATCH (WHERE b.eprop1 = 10001)-[b]->(c) COLUMNS (b.ename AS bname, c.vname AS cname)); -- select all the properties across all the labels associated with a given type -- 2.34.1
