From 88b2c581e45fd0c7fc2291c508024c17b354ade2 Mon Sep 17 00:00:00 2001 From: Muhammad Taha Naveed Date: Sat, 28 Oct 2023 00:05:43 +0500 Subject: [PATCH] Fix issue #1045 - error using path var in WHERE (#1295) - Added cypher_path as an entity(along side cypher_node and cypher_relationship) in order for WHERE in MATCH clause to be able to reference it. - Fixed relevant error messages to be more specific. - Added regression tests. --- regress/expected/cypher_match.out | 51 ++++++++++++++++++-- regress/expected/cypher_vle.out | 2 +- regress/sql/cypher_match.sql | 9 ++++ src/backend/parser/cypher_clause.c | 25 ++++++++++ src/backend/parser/cypher_expr.c | 16 +++--- src/backend/parser/cypher_transform_entity.c | 19 ++++++++ src/include/parser/cypher_transform_entity.h | 4 +- 7 files changed, 112 insertions(+), 14 deletions(-) diff --git a/regress/expected/cypher_match.out b/regress/expected/cypher_match.out index 3632d26db..00dcd2d45 100644 --- a/regress/expected/cypher_match.out +++ b/regress/expected/cypher_match.out @@ -2249,15 +2249,15 @@ ERROR: variable "p" is for a path LINE 1: ...ELECT * FROM cypher('cypher_match', $$ MATCH p=()-[p]->() RE... ^ SELECT * FROM cypher('cypher_match', $$ MATCH p=() MATCH (p) RETURN p $$)as (p agtype); -ERROR: variable 'p' already exists +ERROR: variable 'p' is for a path LINE 1: ... FROM cypher('cypher_match', $$ MATCH p=() MATCH (p) RETURN ... ^ SELECT * FROM cypher('cypher_match', $$ MATCH p=() MATCH (p)-[]->() RETURN p $$)as (p agtype); -ERROR: variable 'p' already exists +ERROR: variable 'p' is for a path LINE 1: ... FROM cypher('cypher_match', $$ MATCH p=() MATCH (p)-[]->() ... ^ SELECT * FROM cypher('cypher_match', $$ MATCH p=() MATCH ()-[p]->() RETURN p $$)as (p agtype); -ERROR: variable 'p' already exists +ERROR: variable 'p' is for a path LINE 1: ...ROM cypher('cypher_match', $$ MATCH p=() MATCH ()-[p]->() RE... ^ SELECT * FROM cypher('cypher_match', $$ MATCH (p) MATCH p=() RETURN p $$)as (p agtype); @@ -2784,6 +2784,51 @@ SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (b:Part) RETURN count( 16 (1 row) +-- +-- Issue 1045 +-- +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() RETURN length(p) $$) as (length agtype); + length +-------- + 1 + 2 + 1 + 1 + 1 + 2 + 1 + 2 +(8 rows) + +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() WHERE length(p) > 1 RETURN length(p) $$) as (length agtype); + length +-------- + 2 + 2 + 2 +(3 rows) + +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() WHERE size(nodes(p)) = 3 RETURN nodes(p)[0] $$) as (nodes agtype); + nodes +----------------------------------------------------------------------------------------------------- + {"id": 281474976710660, "label": "_ag_label_vertex", "properties": {"age": 4, "name": "F"}}::vertex + {"id": 281474976710667, "label": "_ag_label_vertex", "properties": {"name": "Dave"}}::vertex + {"id": 281474976710668, "label": "_ag_label_vertex", "properties": {"name": "John"}}::vertex +(3 rows) + +SELECT * FROM cypher('cypher_match', $$ MATCH (n {name:'Dave'}) MATCH p=()-[*]->() WHERE nodes(p)[0] = n RETURN length(p) $$) as (length agtype); + length +-------- + 1 + 2 +(2 rows) + +SELECT * FROM cypher('cypher_match', $$ MATCH p1=(n {name:'Dave'})-[]->() MATCH p2=()-[*]->() WHERE p2=p1 RETURN p2=p1 $$) as (path agtype); + path +------ + true +(1 row) + -- -- Clean up -- diff --git a/regress/expected/cypher_vle.out b/regress/expected/cypher_vle.out index d9c2091d9..74d392e47 100644 --- a/regress/expected/cypher_vle.out +++ b/regress/expected/cypher_vle.out @@ -807,7 +807,7 @@ ERROR: variable 'p' is for a VLE edge LINE 1: ... cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH (p) RETURN ... ^ SELECT * FROM cypher('cypher_vle', $$ MATCH p=() MATCH ()-[p *]-() RETURN p $$)as (p agtype); -ERROR: variable 'p' already exists +ERROR: variable 'p' is for a path LINE 1: ... FROM cypher('cypher_vle', $$ MATCH p=() MATCH ()-[p *]-() R... ^ SELECT * FROM cypher('cypher_vle', $$ MATCH ()-[p *]-() MATCH p=() RETURN p $$)as (p agtype); diff --git a/regress/sql/cypher_match.sql b/regress/sql/cypher_match.sql index b9a766afa..7da54e9d2 100644 --- a/regress/sql/cypher_match.sql +++ b/regress/sql/cypher_match.sql @@ -1169,6 +1169,15 @@ SELECT * FROM cypher('issue_945', $$ MATCH (:Part) MATCH (b:Part) RETURN count(* SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (b:Part) RETURN count(*) $$) as (result agtype); +-- +-- Issue 1045 +-- +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() RETURN length(p) $$) as (length agtype); +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() WHERE length(p) > 1 RETURN length(p) $$) as (length agtype); +SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() WHERE size(nodes(p)) = 3 RETURN nodes(p)[0] $$) as (nodes agtype); +SELECT * FROM cypher('cypher_match', $$ MATCH (n {name:'Dave'}) MATCH p=()-[*]->() WHERE nodes(p)[0] = n RETURN length(p) $$) as (length agtype); +SELECT * FROM cypher('cypher_match', $$ MATCH p1=(n {name:'Dave'})-[]->() MATCH p2=()-[*]->() WHERE p2=p1 RETURN p2=p1 $$) as (path agtype); + -- -- Clean up -- diff --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c index b3f8bf3fa..c38570cf5 100644 --- a/src/backend/parser/cypher_clause.c +++ b/src/backend/parser/cypher_clause.c @@ -3907,6 +3907,13 @@ static transform_entity *transform_VLE_edge_entity(cypher_parsestate *cpstate, errmsg("variable '%s' is for an edge", rel->name), parser_errposition(pstate, rel->location))); } + else if (entity->type == ENT_PATH) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable '%s' is for a path", rel->name), + parser_errposition(pstate, rel->location))); + } else { ereport(ERROR, @@ -4497,9 +4504,13 @@ transform_match_create_path_variable(cypher_parsestate *cpstate, /* otherwise, build the expr node for the function */ else { + transform_entity *entity; expr = (Expr*)makeFuncExpr(build_path_oid, AGTYPEOID, entity_exprs, InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL); + + entity = make_transform_entity(cpstate, ENT_PATH, (Node *)path, expr); + cpstate->entities = lappend(cpstate->entities, entity); } resno = cpstate->pstate.p_next_resno++; @@ -4668,6 +4679,13 @@ static Expr *transform_cypher_edge(cypher_parsestate *cpstate, errmsg("variable '%s' is for a VLE edge", rel->name), parser_errposition(pstate, rel->location))); } + else if (entity->type == ENT_PATH) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable '%s' is for a path", rel->name), + parser_errposition(pstate, rel->location))); + } } else if (te && !entity) @@ -4907,6 +4925,13 @@ static Expr *transform_cypher_node(cypher_parsestate *cpstate, errmsg("variable '%s' is for a VLE edge", node->name), parser_errposition(pstate, node->location))); } + else if (entity->type == ENT_PATH) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_ALIAS), + errmsg("variable '%s' is for a path", node->name), + parser_errposition(pstate, node->location))); + } } /* If their is a te but no entity, it implies that their is diff --git a/src/backend/parser/cypher_expr.c b/src/backend/parser/cypher_expr.c index 15bfc8127..a3ea59c18 100644 --- a/src/backend/parser/cypher_expr.c +++ b/src/backend/parser/cypher_expr.c @@ -341,17 +341,15 @@ static Node *transform_ColumnRef(cypher_parsestate *cpstate, ColumnRef *cref) } /* - * If expr_kind is WHERE, Try to find the columnRef as a - * transform_entity and extract the expr. + * Try to find the columnRef as a transform_entity + * and extract the expr. */ - if (pstate->p_expr_kind == EXPR_KIND_WHERE) + te = find_variable(cpstate, colname) ; + if (te != NULL && te->expr != NULL && + te->declared_in_current_clause) { - te = find_variable(cpstate, colname) ; - if (te != NULL && te->expr != NULL) - { - node = (Node *)te->expr; - break; - } + node = (Node *)te->expr; + break; } /* * Not known as a column of any range-table entry. diff --git a/src/backend/parser/cypher_transform_entity.c b/src/backend/parser/cypher_transform_entity.c index b80c22811..6e5a4e22b 100644 --- a/src/backend/parser/cypher_transform_entity.c +++ b/src/backend/parser/cypher_transform_entity.c @@ -39,6 +39,10 @@ transform_entity *make_transform_entity(cypher_parsestate *cpstate, { entity->entity.rel = (cypher_relationship *)node; } + else if (entity->type == ENT_PATH) + { + entity->entity.path = (cypher_path *)node; + } else { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -90,6 +94,13 @@ transform_entity *find_transform_entity(cypher_parsestate *cpstate, return entity; } } + else if (type == ENT_PATH) + { + if (entity->entity.path->var_name != NULL && !strcmp(entity->entity.path->var_name, name)) + { + return entity; + } + } } return NULL; @@ -116,6 +127,10 @@ transform_entity *find_variable(cypher_parsestate *cpstate, char *name) { entity_name = entity->entity.rel->name; } + else if (entity->type == ENT_PATH) + { + entity_name = entity->entity.path->var_name; + } else { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -142,6 +157,10 @@ char *get_entity_name(transform_entity *entity) { return entity->entity.node->name; } + else if (entity->type == ENT_PATH) + { + return entity->entity.path->var_name; + } else { ereport(ERROR, diff --git a/src/include/parser/cypher_transform_entity.h b/src/include/parser/cypher_transform_entity.h index 79a8a336d..0d49430ea 100644 --- a/src/include/parser/cypher_transform_entity.h +++ b/src/include/parser/cypher_transform_entity.h @@ -31,7 +31,8 @@ enum transform_entity_type { ENT_VERTEX = 0x0, ENT_EDGE, - ENT_VLE_EDGE + ENT_VLE_EDGE, + ENT_PATH }; enum transform_entity_join_side @@ -83,6 +84,7 @@ typedef struct { cypher_node *node; cypher_relationship *rel; + cypher_path *path; } entity; } transform_entity;