From 05db8d8ec0919c3b705bd69138e93449aad2a90b Mon Sep 17 00:00:00 2001 From: Muhammad Taha Naveed Date: Thu, 8 Jun 2023 15:52:06 +0500 Subject: [PATCH] Fix issue #975 Invalid reuse of variables in CREATE clause - Added checks for invalid use of variables in create clause. - Fixed error position of path and node variables. - Added regression tests. --- regress/expected/cypher_create.out | 144 +++++++++++++++++++++++++++-- regress/expected/cypher_match.out | 30 +++--- regress/sql/cypher_create.sql | 86 +++++++++++++++++ src/backend/parser/cypher_clause.c | 84 +++++++++++++---- src/backend/parser/cypher_gram.y | 3 +- 5 files changed, 307 insertions(+), 40 deletions(-) diff --git a/regress/expected/cypher_create.out b/regress/expected/cypher_create.out index 7cac75e35..8bef1d05e 100644 --- a/regress/expected/cypher_create.out +++ b/regress/expected/cypher_create.out @@ -499,8 +499,8 @@ SELECT * FROM cypher('cypher_create', $$ CREATE (a {test:1})-[:e_var]->() $$) as (a agtype); ERROR: previously declared nodes in a create clause cannot have properties -LINE 1: SELECT * FROM cypher('cypher_create', $$ - ^ +LINE 4: CREATE (a {test:1})-[:e_var]->() + ^ -- Var 'a' cannot change labels SELECT * FROM cypher('cypher_create', $$ MATCH (a:n_var) @@ -508,16 +508,16 @@ SELECT * FROM cypher('cypher_create', $$ CREATE (a:new_label)-[:e_var]->() $$) as (a agtype); ERROR: previously declared variables cannot have a label -LINE 1: SELECT * FROM cypher('cypher_create', $$ - ^ +LINE 4: CREATE (a:new_label)-[:e_var]->() + ^ SELECT * FROM cypher('cypher_create', $$ MATCH (a:n_var)-[b]-() WHERE a.name = 'Node A' CREATE (a)-[b:e_var]->() $$) as (a agtype); ERROR: variable b already exists -LINE 1: SELECT * FROM cypher('cypher_create', $$ - ^ +LINE 4: CREATE (a)-[b:e_var]->() + ^ -- A valid single vertex path SELECT * FROM cypher('cypher_create', $$ CREATE p=(a) @@ -578,7 +578,7 @@ SELECT * FROM cypher('cypher_create', $$ $$) as (a agtype); ERROR: label existing_elabel is for edges, not vertices LINE 2: CREATE (a:existing_elabel { id: 5}) - ^ + ^ -- -- check the cypher CREATE clause inside an INSERT INTO -- @@ -641,12 +641,137 @@ SELECT * FROM cypher('cypher_create', $$ MATCH (a:Part) RETURN a $$) as (a agtyp END; -- +-- variable reuse +-- +-- Valid variable reuse +SELECT * FROM cypher('cypher_create', $$ + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + n1 | e | n2 +----------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------+---------------------------------------------------------------- + {"id": 281474976710681, "label": "", "properties": {}}::vertex | {"id": 3940649673949185, "label": "new", "end_id": 281474976710681, "start_id": 281474976710681, "properties": {}}::edge | {"id": 281474976710681, "label": "", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:node)-[e:new]->(p) + RETURN p,e,p +$$) as (n1 agtype, e agtype, n2 agtype); + n1 | e | n2 +---------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------- + {"id": 4222124650659841, "label": "node", "properties": {}}::vertex | {"id": 3940649673949186, "label": "new", "end_id": 4222124650659841, "start_id": 4222124650659841, "properties": {}}::edge | {"id": 4222124650659841, "label": "node", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + n1 | e | n2 +----------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------+---------------------------------------------------------------- + {"id": 281474976710682, "label": "", "properties": {}}::vertex | {"id": 3940649673949187, "label": "new", "end_id": 281474976710682, "start_id": 281474976710682, "properties": {}}::edge | {"id": 281474976710682, "label": "", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:n1) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + n1 | e | n2 +-------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------- + {"id": 4503599627370497, "label": "n1", "properties": {}}::vertex | {"id": 3940649673949188, "label": "new", "end_id": 4503599627370497, "start_id": 4503599627370497, "properties": {}}::edge | {"id": 4503599627370497, "label": "n1", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('cypher_create', $$ + MATCH (p:node) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + n1 | e | n2 +---------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------- + {"id": 4222124650659841, "label": "node", "properties": {}}::vertex | {"id": 3940649673949189, "label": "new", "end_id": 4222124650659841, "start_id": 4222124650659841, "properties": {}}::edge | {"id": 4222124650659841, "label": "node", "properties": {}}::vertex +(1 row) + +-- Invalid variable reuse +SELECT * FROM cypher('cypher_create', $$ + CREATE (p)-[a:new]->(p {n0:'n1'}) +$$) as (a agtype); +ERROR: previously declared nodes in a create clause cannot have properties +LINE 2: CREATE (p)-[a:new]->(p {n0:'n1'}) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:n0)-[a:new]->(p:n1) +$$) as (a agtype); +ERROR: previously declared variables cannot have a label +LINE 2: CREATE (p:n0)-[a:new]->(p:n1) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(p) +$$) as (a agtype); +ERROR: variable "p" already exists +LINE 2: CREATE p=(p) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE p=() CREATE (p) +$$) as (a agtype); +ERROR: agtype must resolve to a vertex +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(a)-[p:b]->(a) +$$) as (a agtype); +ERROR: variable "p" already exists +LINE 2: CREATE p=(a)-[p:b]->(a) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(a)-[:new]->(p) +$$) as (a agtype); +ERROR: variable "p" already exists +LINE 2: CREATE p=(a)-[:new]->(p) + ^ +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE p=() +$$) as (a agtype); +ERROR: variable "p" already exists +LINE 2: MATCH (p) CREATE p=() + ^ +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE p=(p) +$$) as (a agtype); +ERROR: variable "p" already exists +LINE 2: MATCH (p) CREATE p=(p) + ^ +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE (a)-[p:b]->(a) +$$) as (a agtype); +ERROR: variable p already exists +LINE 2: MATCH (p) CREATE (a)-[p:b]->(a) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE (a)-[e:new]->(p)-[e]->(a) +$$) as (a agtype); +ERROR: relationships must be specify a label in CREATE. +LINE 2: CREATE (a)-[e:new]->(p)-[e]->(a) + ^ +SELECT * FROM cypher('cypher_create', $$ + CREATE (a)-[e:new]->(p) + CREATE (p)-[e:new]->(a) +$$) as (a agtype); +ERROR: variable e already exists +LINE 3: CREATE (p)-[e:new]->(a) + ^ +SELECT * FROM cypher('cypher_create', $$ + MATCH (a)-[e:new]->(p) + CREATE (p)-[e:new]->(a) +$$) as (a agtype); +ERROR: variable e already exists +LINE 3: CREATE (p)-[e:new]->(a) + ^ +-- -- Clean up -- DROP TABLE simple_path; DROP FUNCTION create_test; SELECT drop_graph('cypher_create', true); -NOTICE: drop cascades to 13 other objects +NOTICE: drop cascades to 16 other objects DETAIL: drop cascades to table cypher_create._ag_label_vertex drop cascades to table cypher_create._ag_label_edge drop cascades to table cypher_create.v @@ -660,6 +785,9 @@ drop cascades to table cypher_create.existing_vlabel drop cascades to table cypher_create.existing_elabel drop cascades to table cypher_create.knows drop cascades to table cypher_create."Part" +drop cascades to table cypher_create.new +drop cascades to table cypher_create.node +drop cascades to table cypher_create.n1 NOTICE: graph "cypher_create" has been dropped drop_graph ------------ diff --git a/regress/expected/cypher_match.out b/regress/expected/cypher_match.out index da5e45122..50eebeb43 100644 --- a/regress/expected/cypher_match.out +++ b/regress/expected/cypher_match.out @@ -508,43 +508,43 @@ SELECT * FROM cypher('cypher_match', $$ $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a)-[]-()-[]-(a:v1) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a)-[]-(a:v2)-[]-(a) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a)-[]-(a:v2)-[]-(a) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a)-[]-(a:v1) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a)-[]-(a:v1) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a)-[]-(a)-[]-(a:v1) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a)-[]-(a)-[]-(a:v1) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a)-[]-(a)-[]-(a:invalid_label) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a)-[]-(a)-[]-(a:invalid_label) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a) MATCH (a:v1) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a) MATCH (a:v1) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a) MATCH (a:invalid_label) RETURN a $$) AS (a agtype); ERROR: multiple labels for variable 'a' are not supported LINE 2: MATCH (a) MATCH (a:invalid_label) RETURN a - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (a:v1)-[]-()-[a]-() RETURN a $$) AS (a agtype); @@ -718,8 +718,8 @@ SELECT * FROM cypher('cypher_match', $$ MATCH (r1:invalid)-[]->(r1)-[]->(r1)-[]->(r1:invalids) return r1 $$) AS (r1 agtype); ERROR: multiple labels for variable 'r1' are not supported -LINE 2: MATCH (r1:invalid)-[]->(r1)-[]->(r1)-[]->(r1:invali... - ^ +LINE 2: ... MATCH (r1:invalid)-[]->(r1)-[]->(r1)-[]->(r1:invalid... + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (r1:invalid)-[]->(r1)-[]->(r1)-[]->(r1)-[r1]->() return r1 $$) AS (r1 agtype); @@ -732,25 +732,25 @@ SELECT * FROM cypher('cypher_match', $$ $$) AS (r1 agtype); ERROR: multiple labels for variable 'r1' are not supported LINE 2: MATCH (r1:e1), (r1:e2) return r1 - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (r1:invalid), (r1:e2) return r1 $$) AS (r1 agtype); ERROR: multiple labels for variable 'r1' are not supported LINE 2: MATCH (r1:invalid), (r1:e2) return r1 - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (r1:e1), (r1:invalid) return r1 $$) AS (r1 agtype); ERROR: multiple labels for variable 'r1' are not supported LINE 2: MATCH (r1:e1), (r1:invalid) return r1 - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (r1:e1), (r1), (r1:invalid) return r1 $$) AS (r1 agtype); ERROR: multiple labels for variable 'r1' are not supported LINE 2: MATCH (r1:e1), (r1), (r1:invalid) return r1 - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH (r0)-[r0]->() MATCH ()-[]->() RETURN r0 $$) AS (r0 agtype); @@ -768,7 +768,7 @@ SELECT * FROM cypher('cypher_match', $$ $$) AS (r0 agtype); ERROR: variable 'r0' is for a edge LINE 2: MATCH ()-[r0]->() MATCH ()-[]->(r0) RETURN r0 - ^ + ^ SELECT * FROM cypher('cypher_match', $$ MATCH ()-[r0:e1]->() MATCH ()-[r0:e2]->() RETURN r0 $$) AS (r0 agtype); @@ -1083,7 +1083,7 @@ SELECT * FROM cypher('cypher_match', AS (u agtype, e agtype, v agtype); ERROR: variable `x` does not exist LINE 2: $$MATCH (u)-[e]->(v) WHERE EXISTS((u)-[e]->(x)) RETURN u, e... - ^ + ^ -- -- Tests for EXISTS(property) -- diff --git a/regress/sql/cypher_create.sql b/regress/sql/cypher_create.sql index 5934dcb18..dd9506982 100644 --- a/regress/sql/cypher_create.sql +++ b/regress/sql/cypher_create.sql @@ -299,6 +299,92 @@ SELECT * FROM cypher('cypher_create', $$ CREATE (a:Part {part_num: '673'}) $$) a SELECT * FROM cypher('cypher_create', $$ MATCH (a:Part) RETURN a $$) as (a agtype); END; +-- +-- variable reuse +-- + +-- Valid variable reuse + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:node)-[e:new]->(p) + RETURN p,e,p +$$) as (n1 agtype, e agtype, n2 agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:n1) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + +SELECT * FROM cypher('cypher_create', $$ + MATCH (p:node) + CREATE (p)-[a:new]->(p) + RETURN p,a,p +$$) as (n1 agtype, e agtype, n2 agtype); + +-- Invalid variable reuse + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p)-[a:new]->(p {n0:'n1'}) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (p:n0)-[a:new]->(p:n1) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(p) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE p=() CREATE (p) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(a)-[p:b]->(a) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE p=(a)-[:new]->(p) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE p=() +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE p=(p) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + MATCH (p) CREATE (a)-[p:b]->(a) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (a)-[e:new]->(p)-[e]->(a) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + CREATE (a)-[e:new]->(p) + CREATE (p)-[e:new]->(a) +$$) as (a agtype); + +SELECT * FROM cypher('cypher_create', $$ + MATCH (a)-[e:new]->(p) + CREATE (p)-[e:new]->(a) +$$) as (a agtype); + -- -- Clean up -- diff --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c index 493206c30..910104bcf 100644 --- a/src/backend/parser/cypher_clause.c +++ b/src/backend/parser/cypher_clause.c @@ -4893,6 +4893,18 @@ transform_cypher_create_path(cypher_parsestate *cpstate, List **target_list, ccp->path_attr_num = InvalidAttrNumber; + if (in_path) + { + if (findTarget(*target_list, path->var_name) != NULL) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable \"%s\" already exists", + path->var_name), + parser_errposition(pstate, path->location))); + } + } + foreach (lc, path->path) { if (is_ag_node(lfirst(lc), cypher_node)) @@ -4904,7 +4916,18 @@ transform_cypher_create_path(cypher_parsestate *cpstate, List **target_list, transform_create_cypher_node(cpstate, target_list, node); if (in_path) + { + if (node->name && strcmp(node->name, path->var_name) == 0) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable \"%s\" already exists", + path->var_name), + parser_errposition(pstate, path->location))); + } rel->flags |= CYPHER_TARGET_NODE_IN_PATH_VAR; + } + transformed_path = lappend(transformed_path, rel); @@ -4922,7 +4945,17 @@ transform_cypher_create_path(cypher_parsestate *cpstate, List **target_list, transform_create_cypher_edge(cpstate, target_list, edge); if (in_path) + { + if (edge->name && strcmp(edge->name, path->var_name) == 0) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable \"%s\" already exists", + path->var_name), + parser_errposition(pstate, path->location))); + } rel->flags |= CYPHER_TARGET_NODE_IN_PATH_VAR; + } transformed_path = lappend(transformed_path, rel); @@ -4934,7 +4967,7 @@ transform_cypher_create_path(cypher_parsestate *cpstate, List **target_list, else { ereport(ERROR, - (errmsg_internal("unrecognized node in create pattern"))); + (errmsg_internal("unrecognized node in create pattern"))); } } @@ -4982,10 +5015,7 @@ transform_create_cypher_edge(cypher_parsestate *cpstate, List **target_list, if (edge->label) { - label_cache_data *lcd = - search_label_name_graph_cache(edge->label, cpstate->graph_oid); - - if (lcd && lcd->kind != LABEL_KIND_EDGE) + if (get_label_kind(edge->label, cpstate->graph_oid) == LABEL_KIND_VERTEX) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -5005,11 +5035,16 @@ transform_create_cypher_edge(cypher_parsestate *cpstate, List **target_list, * Variables can be declared in a CREATE clause, but not used if * it already exists. */ - if (variable_exists(cpstate, edge->name)) + transform_entity *entity; + + entity = find_variable(cpstate, edge->name); + + if ((entity && entity->type != ENT_EDGE) || variable_exists(cpstate, edge->name)) { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("variable %s already exists", edge->name))); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("variable %s already exists", edge->name), + parser_errposition(pstate, edge->location))); } rel->variable_name = edge->name; @@ -5122,10 +5157,7 @@ transform_create_cypher_node(cypher_parsestate *cpstate, List **target_list, if (node->label) { - label_cache_data *lcd = - search_label_name_graph_cache(node->label, cpstate->graph_oid); - - if (lcd && lcd->kind != LABEL_KIND_VERTEX) + if (get_label_kind(node->label, cpstate->graph_oid) == LABEL_KIND_EDGE) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("label %s is for edges, not vertices", @@ -5142,9 +5174,16 @@ transform_create_cypher_node(cypher_parsestate *cpstate, List **target_list, { transform_entity *entity; + TargetEntry *te = findTarget(*target_list, node->name); entity = find_variable(cpstate, node->name); - if (entity) + /* + * If we find an entity as well as a target Entry with same name, + * that means that the variable is either vertex, edge or vle. + * but if we find a target entry but not an entity that means + * that the variable can be other than vertex, edge or vle e.g path. + */ + if (entity && te) { if (entity->type != ENT_VERTEX) { @@ -5157,6 +5196,16 @@ transform_create_cypher_node(cypher_parsestate *cpstate, List **target_list, return transform_create_cypher_existing_node(cpstate, target_list, entity->declared_in_current_clause, node); } + else if (te) + { + /* + * Here we are not sure if the te is a vertex, path or something + * else. So we will let it pass and the execution stage will catch + * the error if variable was not vertex. + */ + return transform_create_cypher_existing_node(cpstate, target_list, + te, node); + } } // otherwise transform the target node as a new node @@ -5197,6 +5246,7 @@ static cypher_target_node *transform_create_cypher_existing_node( cypher_node *node) { cypher_target_node *rel = make_ag_node(cypher_target_node); + ParseState *pstate = (ParseState *)cpstate; rel->type = LABEL_KIND_VERTEX; rel->flags = CYPHER_TARGET_NODE_FLAG_NONE; @@ -5208,13 +5258,15 @@ static cypher_target_node *transform_create_cypher_existing_node( { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("previously declared nodes in a create clause cannot have properties"))); + errmsg("previously declared nodes in a create clause cannot have properties"), + parser_errposition(pstate, node->location))); } if (node->label) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("previously declared variables cannot have a label"))); + errmsg("previously declared variables cannot have a label"), + parser_errposition(pstate, node->location))); } /* * When the variable is declared in the same clause this vertex is a part of diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y index 4fb599a73..2fd057d6e 100644 --- a/src/backend/parser/cypher_gram.y +++ b/src/backend/parser/cypher_gram.y @@ -1115,6 +1115,7 @@ path: p = (cypher_path *)$3; p->var_name = $1; + p->location = @1; $$ = (Node *)p; } @@ -1182,7 +1183,7 @@ path_node: n->label = $3; n->parsed_label = $3; n->props = $4; - n->location = @1; + n->location = @2; $$ = (Node *)n; }