From 42e83476b0f6ae18d7174447a8b0825e16d9ef31 Mon Sep 17 00:00:00 2001 From: John Gemignani Date: Thu, 16 Nov 2023 15:27:13 -0800 Subject: [PATCH] Fix issue #1399 EXISTS doesn't handle non-existent labels (#1400) Fixed issue #1399 where EXISTS doesn't handle non-existent labels. However, the issue actually is due to the extra processing (read: extra distance between the AST nodes) that prevented the transform logic from being able to resolve variables created in previous clauses further than one parent up. Added logic to allow the `find_variable` function to search up the parent parse nodes. Added regression tests to cover these cases. --- regress/expected/cypher_match.out | 172 +++++++++++++++++++ regress/sql/cypher_match.sql | 86 ++++++++++ src/backend/parser/cypher_transform_entity.c | 53 +++--- 3 files changed, 288 insertions(+), 23 deletions(-) diff --git a/regress/expected/cypher_match.out b/regress/expected/cypher_match.out index 00dcd2d45..8c427497d 100644 --- a/regress/expected/cypher_match.out +++ b/regress/expected/cypher_match.out @@ -2829,6 +2829,167 @@ SELECT * FROM cypher('cypher_match', $$ MATCH p1=(n {name:'Dave'})-[]->() MATCH true (1 row) +-- +-- Issue 1399 EXISTS leads to an error if a relation label does not exists as database table +-- +SELECT create_graph('issue_1399'); +NOTICE: graph "issue_1399" has been created + create_graph +-------------- + +(1 row) + +-- this is an empty graph so these should return 0 +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); + c +--- +(0 rows) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); + c +--- +(0 rows) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); + c +--- +(0 rows) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); + c +--- +(0 rows) + +-- this is an empty graph so these should return false +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN count(foo) > 0 +$$) as (c agtype); + c +------- + false +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN count(foo) > 0 +$$) as (c agtype); + c +------- + false +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN count(foo) > 0 +$$) as (c agtype); + c +------- + false +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN count(foo) > 0 +$$) as (c agtype); + c +------- + false +(1 row) + +-- create 1 path +SELECT * FROM cypher('issue_1399', $$ + CREATE (foo)-[:BAR]->() RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710657, "label": "", "properties": {}}::vertex +(1 row) + +-- these should each return 1 row as it is a directed edge and +-- only one vertex can match. +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710657, "label": "", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710658, "label": "", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710657, "label": "", "properties": {}}::vertex +(1 row) + +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710658, "label": "", "properties": {}}::vertex +(1 row) + +-- this should return 0 rows as it can't exist - that path isn't in BAR2 +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR2]->()) + RETURN foo +$$) as (c agtype); + c +--- +(0 rows) + +-- this should return 2 rows as they all exist +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR2]->()) + RETURN foo +$$) as (c agtype); + c +---------------------------------------------------------------- + {"id": 281474976710657, "label": "", "properties": {}}::vertex + {"id": 281474976710658, "label": "", "properties": {}}::vertex +(2 rows) + -- -- Clean up -- @@ -2892,6 +3053,17 @@ NOTICE: graph "issue_945" has been dropped (1 row) +SELECT drop_graph('issue_1399', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table issue_1399._ag_label_vertex +drop cascades to table issue_1399._ag_label_edge +drop cascades to table issue_1399."BAR" +NOTICE: graph "issue_1399" has been dropped + drop_graph +------------ + +(1 row) + -- -- End -- diff --git a/regress/sql/cypher_match.sql b/regress/sql/cypher_match.sql index 7da54e9d2..483f0ef04 100644 --- a/regress/sql/cypher_match.sql +++ b/regress/sql/cypher_match.sql @@ -1178,6 +1178,91 @@ SELECT * FROM cypher('cypher_match', $$ MATCH p=()-[*]->() WHERE size(nodes(p)) 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); +-- +-- Issue 1399 EXISTS leads to an error if a relation label does not exists as database table +-- +SELECT create_graph('issue_1399'); +-- this is an empty graph so these should return 0 +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); +-- this is an empty graph so these should return false +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN count(foo) > 0 +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN count(foo) > 0 +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN count(foo) > 0 +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN count(foo) > 0 +$$) as (c agtype); +-- create 1 path +SELECT * FROM cypher('issue_1399', $$ + CREATE (foo)-[:BAR]->() RETURN foo +$$) as (c agtype); +-- these should each return 1 row as it is a directed edge and +-- only one vertex can match. +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR]->()) + RETURN foo +$$) as (c agtype); +-- this should return 0 rows as it can't exist - that path isn't in BAR2 +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE exists((foo)-[:BAR2]->()) + RETURN foo +$$) as (c agtype); +-- this should return 2 rows as they all exist +SELECT * FROM cypher('issue_1399', $$ + MATCH (foo) + WHERE NOT exists((foo)-[:BAR2]->()) + RETURN foo +$$) as (c agtype); + -- -- Clean up -- @@ -1185,6 +1270,7 @@ SELECT drop_graph('cypher_match', true); SELECT drop_graph('test_retrieve_var', true); SELECT drop_graph('test_enable_containment', true); SELECT drop_graph('issue_945', true); +SELECT drop_graph('issue_1399', true); -- -- End diff --git a/src/backend/parser/cypher_transform_entity.c b/src/backend/parser/cypher_transform_entity.c index 6e5a4e22b..5a2508c3c 100644 --- a/src/backend/parser/cypher_transform_entity.c +++ b/src/backend/parser/cypher_transform_entity.c @@ -114,33 +114,40 @@ transform_entity *find_variable(cypher_parsestate *cpstate, char *name) { ListCell *lc; - foreach (lc, cpstate->entities) + /* while we have cypher_parsestates to check */ + while (cpstate) { - transform_entity *entity = lfirst(lc); - char *entity_name; - - if (entity->type == ENT_VERTEX) - { - entity_name = entity->entity.node->name; - } - else if (entity->type == ENT_EDGE || entity->type == ENT_VLE_EDGE) - { - entity_name = entity->entity.rel->name; - } - else if (entity->type == ENT_PATH) + foreach (lc, cpstate->entities) { - entity_name = entity->entity.path->var_name; - } - else - { - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("unknown entity type"))); - } + transform_entity *entity = lfirst(lc); + char *entity_name = NULL; - if (entity_name != NULL && !strcmp(name, entity_name)) - { - return entity; + if (entity->type == ENT_VERTEX) + { + entity_name = entity->entity.node->name; + } + else if (entity->type == ENT_EDGE || entity->type == ENT_VLE_EDGE) + { + 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), + errmsg("unknown entity type"))); + } + + if (entity_name != NULL && !strcmp(name, entity_name)) + { + return entity; + } } + + /* go up to the next parent parse state */ + cpstate = (cypher_parsestate*)cpstate->pstate.parentParseState; } return NULL;