Skip to content

Commit d428791

Browse files
jrgemignanijrgemignani
committed
update grammar file for maintainability
Consolidated duplicate code, added helper functions, and reviewed the grammar file for issues. NOTE: I used an AI tool to review and cleanup the grammar file. I have reviewed all of the work it did. Improvements: 1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths 2. Consolidated EXPLAIN statement handling into make_explain_stmt helper 3. Extracted WITH clause validation into validate_return_item_aliases helper 4. Created make_default_return_node helper for subquery return-less logic 5. Added grammar_improvements regression test Benefits: - Reduced code duplication by ~150 lines - Improved maintainability with helper functions - Eliminated manual string length calculations (error-prone) All 29 existing regression tests pass Added an additional grammar regression test modified: src/backend/parser/cypher_gram.y modified: Makefile new file: regress/expected/grammar.out new file: regress/sql/grammar.sql
1 parent 0ea9464 commit d428791

4 files changed

Lines changed: 367 additions & 164 deletions

File tree

‎Makefile‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ REGRESS = scan \
112112
name_validation \
113113
jsonb_operators \
114114
list_comprehension \
115-
map_projection
115+
map_projection \
116+
grammar
116117

117118
ifneq ($(EXTRA_TESTS),)
118119
REGRESS += $(EXTRA_TESTS)

‎regress/expected/grammar.out‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
--
20+
-- Set up AGE extension
21+
--
22+
LOAD 'age';
23+
SET search_path TO ag_catalog;
24+
--
25+
-- Test grammar improvements work correctly
26+
--
27+
-- Test EXPLAIN variants (consolidated in grammar)
28+
SELECT create_graph('grammar_test');
29+
NOTICE: graph "grammar_test" has been created
30+
create_graph
31+
--------------
32+
33+
(1 row)
34+
35+
-- Basic EXPLAIN (tests consolidated EXPLAIN statement handling)
36+
SELECT * FROM cypher('grammar_test', $$EXPLAIN CREATE (n:Node {id: 1}) RETURN n$$) AS (result agtype);
37+
QUERY PLAN
38+
-----------------------------------------------------------------------------------------------------
39+
Custom Scan (Cypher Create) (cost=0.00..0.00 rows=0 width=32)
40+
-> Subquery Scan on _age_default_alias_previous_cypher_clause (cost=0.00..0.03 rows=1 width=32)
41+
-> Result (cost=0.00..0.02 rows=1 width=128)
42+
(3 rows)
43+
44+
SELECT * FROM cypher('grammar_test', $$EXPLAIN (costs false) CREATE (n:Node {id: 2}) RETURN n$$) AS (n agtype);
45+
QUERY PLAN
46+
------------------------------------------------------------------
47+
Custom Scan (Cypher Create)
48+
-> Subquery Scan on _age_default_alias_previous_cypher_clause
49+
-> Result
50+
(3 rows)
51+
52+
-- Test WITH clause validation (helper function)
53+
SELECT * FROM cypher('grammar_test', $$
54+
CREATE (a:Person {name: 'Alice', age: 30})
55+
RETURN a
56+
$$) AS (a agtype);
57+
a
58+
-------------------------------------------------------------------------------------------------
59+
{"id": 1125899906842625, "label": "Person", "properties": {"age": 30, "name": "Alice"}}::vertex
60+
(1 row)
61+
62+
SELECT * FROM cypher('grammar_test', $$
63+
MATCH (a:Person)
64+
WITH a.name AS name, a.age AS age
65+
RETURN name, age
66+
$$) AS (name agtype, age agtype);
67+
name | age
68+
---------+-----
69+
"Alice" | 30
70+
(1 row)
71+
72+
-- Test WITH DISTINCT with proper aliasing
73+
SELECT * FROM cypher('grammar_test', $$
74+
MATCH (a:Person)
75+
WITH DISTINCT a.name AS name
76+
RETURN name
77+
$$) AS (name agtype);
78+
name
79+
---------
80+
"Alice"
81+
(1 row)
82+
83+
-- Test subquery with implicit RETURN * (helper function)
84+
SELECT * FROM cypher('grammar_test', $$
85+
CREATE (b:Person {name: 'Bob', age: 25})
86+
WITH b
87+
RETURN EXISTS {
88+
MATCH (p:Person)
89+
WHERE p.name = 'Alice'
90+
} AS alice_exists
91+
$$) AS (alice_exists agtype);
92+
alice_exists
93+
--------------
94+
true
95+
(1 row)
96+
97+
-- Test keyword handling (KEYWORD_STRDUP macro)
98+
SELECT * FROM cypher('grammar_test', $$
99+
MATCH (p:Person)
100+
WHERE p.age > 20
101+
RETURN p
102+
ORDER BY p.age ASC
103+
SKIP 0
104+
LIMIT 10
105+
$$) AS (p agtype);
106+
p
107+
-------------------------------------------------------------------------------------------------
108+
{"id": 1125899906842626, "label": "Person", "properties": {"age": 25, "name": "Bob"}}::vertex
109+
{"id": 1125899906842625, "label": "Person", "properties": {"age": 30, "name": "Alice"}}::vertex
110+
(2 rows)
111+
112+
-- Test all safe keywords work properly
113+
SELECT * FROM cypher('grammar_test', $$
114+
MATCH (p:Person)
115+
RETURN p.name AS name, p.age AS age
116+
ORDER BY age DESCENDING
117+
$$) AS (name agtype, age agtype);
118+
ERROR: could not find rte for age
119+
LINE 4: ORDER BY age DESCENDING
120+
^
121+
HINT: variable age does not exist within scope of usage
122+
-- Clean up
123+
SELECT drop_graph('grammar_test', true);
124+
NOTICE: drop cascades to 4 other objects
125+
DETAIL: drop cascades to table grammar_test._ag_label_vertex
126+
drop cascades to table grammar_test._ag_label_edge
127+
drop cascades to table grammar_test."Node"
128+
drop cascades to table grammar_test."Person"
129+
NOTICE: graph "grammar_test" has been dropped
130+
drop_graph
131+
------------
132+
133+
(1 row)
134+
135+
--
136+
-- End
137+
--

‎regress/sql/grammar.sql‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
--
21+
-- Set up AGE extension
22+
--
23+
LOAD 'age';
24+
SET search_path TO ag_catalog;
25+
26+
--
27+
-- Test grammar improvements work correctly
28+
--
29+
30+
-- Test EXPLAIN variants (consolidated in grammar)
31+
SELECT create_graph('grammar_test');
32+
33+
-- Basic EXPLAIN (tests consolidated EXPLAIN statement handling)
34+
SELECT * FROM cypher('grammar_test', $$EXPLAIN CREATE (n:Node {id: 1}) RETURN n$$) AS (result agtype);
35+
SELECT * FROM cypher('grammar_test', $$EXPLAIN (costs false) CREATE (n:Node {id: 2}) RETURN n$$) AS (n agtype);
36+
37+
-- Test WITH clause validation (helper function)
38+
SELECT * FROM cypher('grammar_test', $$
39+
CREATE (a:Person {name: 'Alice', age: 30})
40+
RETURN a
41+
$$) AS (a agtype);
42+
43+
SELECT * FROM cypher('grammar_test', $$
44+
MATCH (a:Person)
45+
WITH a.name AS name, a.age AS age
46+
RETURN name, age
47+
$$) AS (name agtype, age agtype);
48+
49+
-- Test WITH DISTINCT with proper aliasing
50+
SELECT * FROM cypher('grammar_test', $$
51+
MATCH (a:Person)
52+
WITH DISTINCT a.name AS name
53+
RETURN name
54+
$$) AS (name agtype);
55+
56+
-- Test subquery with implicit RETURN * (helper function)
57+
SELECT * FROM cypher('grammar_test', $$
58+
CREATE (b:Person {name: 'Bob', age: 25})
59+
WITH b
60+
RETURN EXISTS {
61+
MATCH (p:Person)
62+
WHERE p.name = 'Alice'
63+
} AS alice_exists
64+
$$) AS (alice_exists agtype);
65+
66+
-- Test keyword handling (KEYWORD_STRDUP macro)
67+
SELECT * FROM cypher('grammar_test', $$
68+
MATCH (p:Person)
69+
WHERE p.age > 20
70+
RETURN p
71+
ORDER BY p.age ASC
72+
SKIP 0
73+
LIMIT 10
74+
$$) AS (p agtype);
75+
76+
-- Test all safe keywords work properly
77+
SELECT * FROM cypher('grammar_test', $$
78+
MATCH (p:Person)
79+
RETURN p.name AS name, p.age AS age
80+
ORDER BY age DESCENDING
81+
$$) AS (name agtype, age agtype);
82+
83+
-- Clean up
84+
SELECT drop_graph('grammar_test', true);
85+
86+
--
87+
-- End
88+
--

0 commit comments

Comments
 (0)