Skip to content

Commit 162bf30

Browse files
jrgemignanijrgemignani
authored
Fix Issue 945 - incorrect count(*) return values (#1288) (#1312)
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 dc82c93 commit 162bf30

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
@@ -2230,10 +2230,10 @@ SELECT * FROM cypher('expr', $$ MATCH (v) RETURN v $$) AS (expression agtype);
22302230
SELECT * FROM cypher('expr', $$ MATCH ()-[e]-() RETURN e $$) AS (expression agtype);
22312231
expression
22322232
---------------------------------------------------------------------------------------------------------------------------
2233-
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22342233
{"id": 1407374883553281, "label": "e1", "end_id": 1125899906842627, "start_id": 1125899906842626, "properties": {}}::edge
2235-
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22362234
{"id": 1407374883553281, "label": "e1", "end_id": 1125899906842627, "start_id": 1125899906842626, "properties": {}}::edge
2235+
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
2236+
{"id": 1407374883553282, "label": "e1", "end_id": 1125899906842626, "start_id": 1125899906842625, "properties": {}}::edge
22372237
(4 rows)
22382238

22392239
-- id()
@@ -2242,10 +2242,10 @@ SELECT * FROM cypher('expr', $$
22422242
$$) AS (id agtype);
22432243
id
22442244
------------------
2245-
1407374883553282
22462245
1407374883553281
2247-
1407374883553282
22482246
1407374883553281
2247+
1407374883553282
2248+
1407374883553282
22492249
(4 rows)
22502250

22512251
SELECT * FROM cypher('expr', $$
@@ -2284,10 +2284,10 @@ SELECT * FROM cypher('expr', $$
22842284
$$) AS (start_id agtype);
22852285
start_id
22862286
------------------
2287-
1125899906842625
22882287
1125899906842626
2289-
1125899906842625
22902288
1125899906842626
2289+
1125899906842625
2290+
1125899906842625
22912291
(4 rows)
22922292

22932293
-- should return null
@@ -2317,10 +2317,10 @@ SELECT * FROM cypher('expr', $$
23172317
$$) AS (end_id agtype);
23182318
end_id
23192319
------------------
2320-
1125899906842626
23212320
1125899906842627
2322-
1125899906842626
23232321
1125899906842627
2322+
1125899906842626
2323+
1125899906842626
23242324
(4 rows)
23252325

23262326
-- 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
@@ -4078,7 +4078,7 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
40784078
node->name),
40794079
parser_errposition(pstate, node->location)));
40804080
}
4081-
4081+
40824082
/*
40834083
* Checks the previous clauses to see if the variable already
40844084
* exists.
@@ -4095,6 +4095,22 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
40954095
output_node = (special_VLE_case && !node->name && !node->props) ?
40964096
false :
40974097
INCLUDE_NODE_IN_JOIN_TREE(path, node);
4098+
/*
4099+
* TODO
4100+
*
4101+
* We need to re-evaluate if we want to use output_node or not.
4102+
* If output_node is set to false, then it basically short circuits
4103+
* the match for instances where a variable isn't specified. While,
4104+
* on the surface, this appears to be a good way to improve
4105+
* execution time of commands that won't do anything, it also
4106+
* causes chained commands to not work correctly. This is because
4107+
* a match without a variable will still feed its tuples to the next
4108+
* stage(s). With this set to false, it won't. So we likely need to
4109+
* remove all of the output_node logic. This needs to be reviewed,
4110+
* though. For now, we just set it to true and update the output of
4111+
* the regression tests.
4112+
*/
4113+
output_node = true;
40984114

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

0 commit comments

Comments
 (0)