Skip to content

Commit cab088a

Browse files
jrgemignanijrgemignani
authored
Fix Issue 945 - incorrect count(*) return values (#1288) (#1313)
Fixed issue 945 where count(*) would have incorrect return values. NOTE - We need to re-evaluate if we want to use output_node or not. If output_node is set to false, then it basically short circuits match for instances where a variable isn't specified - MATCH () RETURN 0; MATCH () MATCH () RETURN 0; While, on the surface, this appears to be a good way to improve execution time of commands that won't do anything, it also causes chained commands to not work correctly. This is because a match without a variable will still feed its tuples to the next stage(s) even though they won't necessarily be sent to the output. For example, with count(*) - MATCH () RETURN count(*); MATCH () MATCH RETURN count(*); With output_node set to false, it won't send the tuples. We will likely need to remove all of the output_node logic. However, this needs to be reviewed. For now, we just set it to true and update the output of the regression tests. Updated regression tests to accomodate the change. Added new regression tests to cover overlooked cases.
1 parent 80e5879 commit cab088a

6 files changed

Lines changed: 312 additions & 46 deletions

File tree

‎regress/expected/cypher_delete.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,8 +707,8 @@ SELECT * FROM cypher('detach_delete', $$ MATCH ()-[e]->() RETURN e.name $$) as (
707707
-------
708708
"ab"
709709
"cd"
710-
"nm"
711710
"am"
711+
"nm"
712712
"pq"
713713
(5 rows)
714714

‎regress/expected/cypher_match.out‎

Lines changed: 247 additions & 35 deletions
Large diffs are not rendered by default.

‎regress/expected/cypher_set.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,8 @@ SELECT * FROM cypher('cypher_set', $$MATCH ()-[n]->(:other_v) RETURN n$$) AS (a
167167
----------------------------------------------------------------------------------------------------------------------------------------
168168
{"id": 1125899906842625, "label": "e", "end_id": 1407374883553281, "start_id": 281474976710657, "properties": {"i": 3, "j": 20}}::edge
169169
{"id": 1125899906842626, "label": "e", "end_id": 1407374883553282, "start_id": 844424930131969, "properties": {"i": 3, "j": 20}}::edge
170-
{"id": 1125899906842627, "label": "e", "end_id": 1407374883553283, "start_id": 844424930131971, "properties": {"i": 3, "j": 20}}::edge
171170
{"id": 1125899906842628, "label": "e", "end_id": 1407374883553284, "start_id": 844424930131970, "properties": {"i": 3, "j": 20}}::edge
171+
{"id": 1125899906842627, "label": "e", "end_id": 1407374883553283, "start_id": 844424930131971, "properties": {"i": 3, "j": 20}}::edge
172172
(4 rows)
173173

174174
SELECT * FROM cypher('cypher_set', $$

‎regress/expected/expr.out‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,10 +2222,10 @@ SELECT * FROM cypher('expr', $$ MATCH (v) RETURN v $$) AS (expression agtype);
22222222
SELECT * FROM cypher('expr', $$ MATCH ()-[e]-() RETURN e $$) AS (expression agtype);
22232223
expression
22242224
---------------------------------------------------------------------------------------------------------------------------
2225-
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22262225
{"id": 1407374883553281, "label": "e1", "end_id": 1125899906842627, "start_id": 1125899906842626, "properties": {}}::edge
2227-
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22282226
{"id": 1407374883553281, "label": "e1", "end_id": 1125899906842627, "start_id": 1125899906842626, "properties": {}}::edge
2227+
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
2228+
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22292229
(4 rows)
22302230

22312231
-- id()
@@ -2234,10 +2234,10 @@ SELECT * FROM cypher('expr', $$
22342234
$$) AS (id agtype);
22352235
id
22362236
------------------
2237-
1407374883553282
22382237
1407374883553281
2239-
1407374883553282
22402238
1407374883553281
2239+
1407374883553282
2240+
1407374883553282
22412241
(4 rows)
22422242

22432243
SELECT * FROM cypher('expr', $$
@@ -2276,10 +2276,10 @@ SELECT * FROM cypher('expr', $$
22762276
$$) AS (start_id agtype);
22772277
start_id
22782278
------------------
2279-
1125899906842625
22802279
1125899906842626
2281-
1125899906842625
22822280
1125899906842626
2281+
1125899906842625
2282+
1125899906842625
22832283
(4 rows)
22842284

22852285
-- should return null
@@ -2309,10 +2309,10 @@ SELECT * FROM cypher('expr', $$
23092309
$$) AS (end_id agtype);
23102310
end_id
23112311
------------------
2312-
1125899906842626
23132312
1125899906842627
2314-
1125899906842626
23152313
1125899906842627
2314+
1125899906842626
2315+
1125899906842626
23162316
(4 rows)
23172317

23182318
-- should return null

‎regress/sql/cypher_match.sql‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,12 +1132,50 @@ SELECT count(*) FROM cypher('test_enable_containment', $$ MATCH (x:Customer {pho
11321132
SELECT count(*) FROM cypher('test_enable_containment', $$ MATCH (x:Customer {phone:[654765876]}) RETURN x $$) as (a agtype);
11331133
SELECT * FROM cypher('test_enable_containment', $$ EXPLAIN (COSTS OFF) MATCH (x:Customer {school:{name:'XYZ',program:{degree:'BSc'}},phone:[987654321],parents:{}}) RETURN x $$) as (a agtype);
11341134

1135+
--
1136+
-- Issue 945
1137+
--
1138+
SELECT create_graph('issue_945');
1139+
SELECT * FROM cypher('issue_945', $$
1140+
CREATE (a:Part {part_num: '123'}),
1141+
(b:Part {part_num: '345'}),
1142+
(c:Part {part_num: '456'}),
1143+
(d:Part {part_num: '789'})
1144+
$$) as (result agtype);
1145+
1146+
-- should match 4
1147+
SELECT * FROM cypher('issue_945', $$
1148+
MATCH (a:Part) RETURN a
1149+
$$) as (result agtype);
1150+
1151+
-- each should return 4
1152+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) RETURN count(*) $$) as (result agtype);
1153+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) RETURN count(*) $$) as (result agtype);
1154+
1155+
-- each should return 4 rows of 0
1156+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) RETURN 0 $$) as (result agtype);
1157+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) RETURN 0 $$) as (result agtype);
1158+
1159+
-- each should return 16 rows of 0
1160+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) MATCH (:Part) RETURN 0 $$) as (result agtype);
1161+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (:Part) RETURN 0 $$) as (result agtype);
1162+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) MATCH (b:Part) RETURN 0 $$) as (result agtype);
1163+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (b:Part) RETURN 0 $$) as (result agtype);
1164+
1165+
-- each should return a count of 16
1166+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) MATCH (:Part) RETURN count(*) $$) as (result agtype);
1167+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (:Part) RETURN count(*) $$) as (result agtype);
1168+
SELECT * FROM cypher('issue_945', $$ MATCH (:Part) MATCH (b:Part) RETURN count(*) $$) as (result agtype);
1169+
SELECT * FROM cypher('issue_945', $$ MATCH (a:Part) MATCH (b:Part) RETURN count(*) $$) as (result agtype);
1170+
1171+
11351172
--
11361173
-- Clean up
11371174
--
11381175
SELECT drop_graph('cypher_match', true);
11391176
SELECT drop_graph('test_retrieve_var', true);
11401177
SELECT drop_graph('test_enable_containment', true);
1178+
SELECT drop_graph('issue_945', true);
11411179

11421180
--
11431181
-- End

‎src/backend/parser/cypher_clause.c‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4054,7 +4054,7 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
40544054
node->name),
40554055
parser_errposition(pstate, node->location)));
40564056
}
4057-
4057+
40584058
/*
40594059
* Checks the previous clauses to see if the variable already
40604060
* exists.
@@ -4071,6 +4071,22 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
40714071
output_node = (special_VLE_case && !node->name && !node->props) ?
40724072
false :
40734073
INCLUDE_NODE_IN_JOIN_TREE(path, node);
4074+
/*
4075+
* TODO
4076+
*
4077+
* We need to re-evaluate if we want to use output_node or not.
4078+
* If output_node is set to false, then it basically short circuits
4079+
* the match for instances where a variable isn't specified. While,
4080+
* on the surface, this appears to be a good way to improve
4081+
* execution time of commands that won't do anything, it also
4082+
* causes chained commands to not work correctly. This is because
4083+
* a match without a variable will still feed its tuples to the next
4084+
* stage(s). With this set to false, it won't. So we likely need to
4085+
* remove all of the output_node logic. This needs to be reviewed,
4086+
* though. For now, we just set it to true and update the output of
4087+
* the regression tests.
4088+
*/
4089+
output_node = true;
40744090

40754091
/* transform vertex */
40764092
expr = transform_cypher_node(cpstate, node, &query->targetList,

0 commit comments

Comments
 (0)