Skip to content

Validate deleted entities #3085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nafraf wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Validate deleted entities #3085

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5ab1506
Validate reuse of deleted entities
nafraf May 17, 2023
86d8476
Validate entities deleted multiple times
nafraf May 18, 2023
a5b7342
Add _DefinedIdentifiers_Free()
nafraf May 18, 2023
4862a56
Validate the use of deleted entities in WITH
nafraf May 19, 2023
4f6b91b
Add _IdentifierDescCloneCallback()
nafraf May 20, 2023
0ca0282
Remove PROJECTED state
nafraf May 21, 2023
9187bbe
Add property contains_deleted to validations_ctx
nafraf May 21, 2023
461d1ad
Add missing raxFree()
nafraf May 21, 2023
9cef2f4
Fix pointer casting
nafraf May 22, 2023
a022cc2
Add _IdentifierDescNew()
nafraf May 22, 2023
7721d0e
Mark identifier states using enumeration
nafraf May 26, 2023
1c4f7b0
Fix validation of returned entities
nafraf May 27, 2023
1ab4845
Update test16_repeated_entity_deletion()
nafraf May 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix validation of returned entities
  • Loading branch information
@nafraf
nafraf committed May 27, 2023
commit 1c4f7b02cbf980394a9a7fc19eeea27617349dce
17 changes: 0 additions & 17 deletions src/ast/ast_validations.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,23 +1687,6 @@ static VISITOR_STRATEGY _Validate_RETURN_Clause
const cypher_astnode_t *alias_node =
cypher_ast_projection_get_alias(proj);

// validate that the returned identifier has not been deleted
if(cypher_astnode_type(expr) == CYPHER_AST_IDENTIFIER) {
// Retrieve "x" from "RETURN x AS a"
const char* identifier_name = cypher_ast_identifier_get_name(expr);
uint len = strlen(identifier_name);

void *identifier = raxFind(vctx->defined_identifiers,
(unsigned char *)identifier_name, len);

if(identifier != raxNotFound && identifier != NULL &&
(identifier == (void *)DELETED_NODE || identifier == (void *)DELETED_EDGE)) {
ErrorCtx_SetError("The bound variable '%.*s' can't be in a RETURN clause because it was deleted.",
len, identifier_name);
return VISITOR_BREAK;
}
}

if(alias_node == NULL) {
continue;
}
Expand Down
55 changes: 20 additions & 35 deletions tests/flow/test_graph_create.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ def test10_match_duplicated_reltype(self):
queries = ["MATCH (a:A)<-[r:R1|R2]-(b:B) RETURN count(r)",
"MATCH (a:A)-[r:R3]->(b:B) RETURN count(r)",
"MATCH (a:A)-[r:R3|R3]->(b:B) RETURN count(r)",
"MATCH (a:A)-[r:R3|R3|R4|R4]->(b:B) RETURN count(r)",
]
"MATCH (a:A)-[r:R3|R3|R4|R4]->(b:B) RETURN count(r)"]
for query in queries:
result = redis_graph.query(query)
expected_result = [[0]]
Expand All @@ -202,44 +201,39 @@ def test11_create_reuse_delete_var(self):
queries = ["MERGE (x) DELETE x CREATE (x)<-[:R]-()",
"CREATE (x) DELETE x CREATE ()<-[:R]-(x)",
"MERGE (a) WITH a AS x DELETE x CREATE ()<-[:R]-(x)",
"CREATE (a) WITH a AS x DELETE x CREATE ()<-[:R]-(x)",
]
"CREATE (a) WITH a AS x DELETE x CREATE ()<-[:R]-(x)"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'x' can't be redeclared in a CREATE clause because it was deleted.")

# test reusing deleted nodes in MERGE
queries = ["MERGE (x) DELETE x MERGE (x)<-[:R]-()",
"CREATE (x) DELETE x MERGE (x)<-[:R]-()",
"MERGE (a) WITH a AS x DELETE x MERGE ()<-[:R]-(x)",
"CREATE (a) WITH a AS x DELETE x MERGE ()<-[:R]-(x)",
]
"CREATE (a) WITH a AS x DELETE x MERGE ()<-[:R]-(x)"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'x' can't be redeclared in a MERGE clause because it was deleted.")

# test reusing deleted nodes in WITH
queries = ["MERGE ()-[:R1]->(x) DELETE x WITH x RETURN 0",
"CREATE ()-[:R1]->(x) DELETE x WITH x RETURN 0",
"MERGE ()<-[:R1]-(x) DELETE x WITH x AS y RETURN 0",
"CREATE ()<-[:R1]-(x) DELETE x WITH x AS y RETURN 0",
]
"CREATE ()<-[:R1]-(x) DELETE x WITH x AS y RETURN 0"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'x' can't be used in a WITH clause because it was deleted.")

# test reusing deleted edges in CREATE
queries = ["MERGE ()-[r:R]->() WITH r AS e DELETE e CREATE (c)<-[e:R]-(d)",
"CREATE ()-[r:R]->() WITH r AS e DELETE e CREATE ()<-[e:R]-()",
]
"CREATE ()-[r:R]->() WITH r AS e DELETE e CREATE ()<-[e:R]-()"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'e' can't be redeclared in a CREATE clause because it was deleted.")

# test reusing deleted edges in MERGE
queries = ["CREATE ()-[r:R]->() WITH r AS e DELETE e MERGE ()<-[e:R]-()",
"MERGE ()-[r:R]->() WITH r AS e DELETE e MERGE (c)<-[e:R]-(d)",
]
"MERGE ()-[r:R]->() WITH r AS e DELETE e MERGE (c)<-[e:R]-(d)"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'e' can't be redeclared in a MERGE clause because it was deleted.")
Expand All @@ -248,36 +242,27 @@ def test11_create_reuse_delete_var(self):
queries = ["MERGE ()-[e:R1]->() DELETE e WITH e RETURN 0",
"CREATE ()-[e:R1]->() DELETE e WITH e RETURN 0",
"MERGE ()-[e:R1]->() DELETE e WITH e AS r RETURN 0",
"CREATE ()-[e:R1]->() DELETE e WITH e AS r RETURN 0",
]
"CREATE ()-[e:R1]->() DELETE e WITH e AS r RETURN 0"]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'e' can't be used in a WITH clause because it was deleted.")

# test reusing deleted nodes/edges in WITH *
queries = ["MERGE (x)-[:R]->() DELETE x WITH * RETURN 0",
"CREATE ()-[:R]->(x) DELETE x WITH * RETURN 0",
"MERGE (a)-[e:R]->(b) DELETE e WITH * RETURN 0",
"CREATE (a)-[e:R]->(b) DELETE e WITH * RETURN 0",
]
"CREATE (a)-[e:R]->(b) DELETE e WITH * RETURN 0"]
for query in queries:
self._assert_exception(redis_graph, query,
"The WITH * clause can't be used because at least one of the bound variables was deleted.")

# TODO: This currently returns the deleted entity
# test returning deleted nodes
queries = ["MERGE (x) DELETE x RETURN x",
"CREATE (x) DELETE x RETURN x",
"MERGE (x)-[r:R]->() DELETE x RETURN x",
"CREATE (x)-[r:R]->() DELETE x RETURN x",
]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'x' can't be in a RETURN clause because it was deleted.")

# test returning deleted edges
queries = ["MERGE ()-[r:R]->() DELETE r RETURN r",
"CREATE ()-[r:R]->() DELETE r RETURN r",
]
for query in queries:
self._assert_exception(redis_graph, query,
"The bound variable 'r' can't be in a RETURN clause because it was deleted.")
# queries = ["MERGE (x) DELETE x RETURN x",
# "CREATE (x) DELETE x RETURN x",
# "MERGE (x)-[r:R]->() DELETE x RETURN x",
# "CREATE (x)-[r:R]->() DELETE x RETURN x"]

# # test returning deleted edges
# queries = ["MERGE ()-[r:R]->() DELETE r RETURN r",
# "CREATE ()-[r:R]->() DELETE r RETURN r"]
1 change: 0 additions & 1 deletion tests/tck/features/clauses/delete/Delete1.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ Feature: Delete1 - Deleting nodes
Then the result should be empty
And no side effects

@skip
Scenario: [5] Ignore null when deleting node
Given an empty graph
When executing query:
Expand Down
1 change: 0 additions & 1 deletion tests/tck/features/clauses/delete/Delete2.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ Feature: Delete2 - Deleting relationships
And the side effects should be:
| -relationships | 1 |

@skip
Scenario: [4] Ignore null when deleting relationship
Given an empty graph
When executing query:
Expand Down