Skip to content

Commit

Permalink
Fix issue #1045 - error using path var in WHERE (#1295)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
MuhammadTahaNaveed authored and jrgemignani committed Dec 13, 2023
1 parent c5b1611 commit a42b055
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 14 deletions.
51 changes: 48 additions & 3 deletions regress/expected/cypher_match.out
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
--
Expand Down
2 changes: 1 addition & 1 deletion regress/expected/cypher_vle.out
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions regress/sql/cypher_match.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
--
Expand Down
25 changes: 25 additions & 0 deletions src/backend/parser/cypher_clause.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions src/backend/parser/cypher_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 19 additions & 0 deletions src/backend/parser/cypher_transform_entity.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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;
Expand All @@ -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),
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/include/parser/cypher_transform_entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -83,6 +84,7 @@ typedef struct
{
cypher_node *node;
cypher_relationship *rel;
cypher_path *path;
} entity;
} transform_entity;

Expand Down

0 comments on commit a42b055

Please sign in to comment.