Fix cross variable references in graph pattern causing 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 <ashutosh.bapat.oss@gmail.com>
Reported-by: Man Zeng <zengman@halodbtech.com>
Reviewed-by: Henson Choi <assam258@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAExHW5u6AoDfNg4%3DR5eVJn_bJn%3DC%3DwVPrto02P_06fxy39fniA%40mail.gmail.com

diff --git a/src/backend/parser/parse_graphtable.c b/src/backend/parser/parse_graphtable.c
index 49ec5c469c2330fa493381b4501e3e3e5c4b5fba..30ddce5aa9f42855241e41689d444efef3539c22 100644 (file)
--- 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 fc2cbeb2083ef2a26c1311e3976c68ce1f04efab..1a7da399f1b5308d04fc785ae27cdbab8b76d9d8 100644 (file)
--- 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 01f480d1a57e62a7797786ff10a1648b67e41584..b579e3df6351398d27b73fc5667ed5988384c324 100644 (file)
--- 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 30e450b384245d4baecc872fcf40370c4b3c036e..4ff98817420db9d5adba18e16913df7a7c8bb61c 100644 (file)
--- 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