This is an automated email from the ASF dual-hosted git repository.
mtaha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/age.git
The following commit(s) were added to refs/heads/master by this push:
new 91de779a Update grammar file for maintainability (#2270)
91de779a is described below
commit 91de779aee0ee241bf8022b4822a9745c8bc9fa3
Author: John Gemignani <[email protected]>
AuthorDate: Thu Dec 11 07:32:13 2025 -0800
Update grammar file for maintainability (#2270)
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
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
modified: src/backend/parser/cypher_gram.y
---
src/backend/parser/cypher_gram.y | 303 ++++++++++++++++++---------------------
1 file changed, 140 insertions(+), 163 deletions(-)
diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y
index 0bafefe1..5ba1e635 100644
--- a/src/backend/parser/cypher_gram.y
+++ b/src/backend/parser/cypher_gram.y
@@ -41,6 +41,9 @@
#define YYMALLOC palloc
#define YYFREE pfree
+
+/* Helper macro for keyword string duplication */
+#define KEYWORD_STRDUP(kw) pnstrdup((kw), strlen(kw))
%}
%locations
@@ -276,6 +279,11 @@ static Node *build_list_comprehension_node(Node *var, Node
*expr,
Node *where, Node *mapping_expr,
int location);
+/* helper functions */
+static ExplainStmt *make_explain_stmt(List *options);
+static void validate_return_item_aliases(List *items, ag_scanner_t scanner);
+static cypher_return *make_default_return_node(int location);
+
%}
%%
@@ -300,81 +308,59 @@ stmt:
* Throw syntax error in this case.
*/
if (yychar != YYEOF)
+ {
yyerror(&yylloc, scanner, extra, "syntax error");
+ }
extra->result = $1;
extra->extra = NULL;
}
| EXPLAIN cypher_stmt semicolon_opt
{
- ExplainStmt *estmt = NULL;
-
if (yychar != YYEOF)
+ {
yyerror(&yylloc, scanner, extra, "syntax error");
-
+ }
extra->result = $2;
-
- estmt = makeNode(ExplainStmt);
- estmt->query = NULL;
- estmt->options = NIL;
- extra->extra = (Node *)estmt;
+ extra->extra = (Node *)make_explain_stmt(NIL);
}
| EXPLAIN VERBOSE cypher_stmt semicolon_opt
{
- ExplainStmt *estmt = NULL;
-
if (yychar != YYEOF)
+ {
yyerror(&yylloc, scanner, extra, "syntax error");
-
+ }
extra->result = $3;
-
- estmt = makeNode(ExplainStmt);
- estmt->query = NULL;
- estmt->options = list_make1(makeDefElem("verbose", NULL, @2));;
- extra->extra = (Node *)estmt;
+ extra->extra = (Node *)make_explain_stmt(
+ list_make1(makeDefElem("verbose", NULL, @2)));
}
| EXPLAIN ANALYZE cypher_stmt semicolon_opt
{
- ExplainStmt *estmt = NULL;
-
if (yychar != YYEOF)
+ {
yyerror(&yylloc, scanner, extra, "syntax error");
-
+ }
extra->result = $3;
-
- estmt = makeNode(ExplainStmt);
- estmt->query = NULL;
- estmt->options = list_make1(makeDefElem("analyze", NULL, @2));;
- extra->extra = (Node *)estmt;
+ extra->extra = (Node *)make_explain_stmt(
+ list_make1(makeDefElem("analyze", NULL, @2)));
}
| EXPLAIN ANALYZE VERBOSE cypher_stmt semicolon_opt
{
- ExplainStmt *estmt = NULL;
-
if (yychar != YYEOF)
yyerror(&yylloc, scanner, extra, "syntax error");
-
extra->result = $4;
-
- estmt = makeNode(ExplainStmt);
- estmt->query = NULL;
- estmt->options = list_make2(makeDefElem("analyze", NULL, @2),
- makeDefElem("verbose", NULL, @3));;
- extra->extra = (Node *)estmt;
+ extra->extra = (Node *)make_explain_stmt(
+ list_make2(makeDefElem("analyze", NULL, @2),
+ makeDefElem("verbose", NULL, @3)));
}
| EXPLAIN '(' utility_option_list ')' cypher_stmt semicolon_opt
{
- ExplainStmt *estmt = NULL;
-
if (yychar != YYEOF)
+ {
yyerror(&yylloc, scanner, extra, "syntax error");
-
+ }
extra->result = $5;
-
- estmt = makeNode(ExplainStmt);
- estmt->query = NULL;
- estmt->options = $3;
- extra->extra = (Node *)estmt;
+ extra->extra = (Node *)make_explain_stmt($3);
}
;
@@ -652,57 +638,20 @@ single_subquery:
single_subquery_no_return:
subquery_part_init reading_clause_list
{
- ColumnRef *cr;
- ResTarget *rt;
cypher_return *n;
/*
* since subqueries allow return-less clauses, we add a
* return node manually to reflect that syntax
*/
- cr = makeNode(ColumnRef);
- cr->fields = list_make1(makeNode(A_Star));
- cr->location = @1;
-
- rt = makeNode(ResTarget);
- rt->name = NULL;
- rt->indirection = NIL;
- rt->val = (Node *)cr;
- rt->location = @1;
-
- n = make_ag_node(cypher_return);
- n->distinct = false;
- n->items = list_make1((Node *)rt);
- n->order_by = NULL;
- n->skip = NULL;
- n->limit = NULL;
-
+ n = make_default_return_node(@1);
$$ = list_concat($1, lappend($2, n));
-
}
- | subquery_pattern
+ | subquery_pattern
{
- ColumnRef *cr;
- ResTarget *rt;
cypher_return *n;
- cr = makeNode(ColumnRef);
- cr->fields = list_make1(makeNode(A_Star));
- cr->location = @1;
-
- rt = makeNode(ResTarget);
- rt->name = NULL;
- rt->indirection = NIL;
- rt->val = (Node *)cr;
- rt->location = @1;
-
- n = make_ag_node(cypher_return);
- n->distinct = false;
- n->items = list_make1((Node *)rt);
- n->order_by = NULL;
- n->skip = NULL;
- n->limit = NULL;
-
+ n = make_default_return_node(@1);
$$ = lappend(list_make1($1), n);
}
;
@@ -958,24 +907,10 @@ limit_opt:
with:
WITH DISTINCT return_item_list order_by_opt skip_opt limit_opt where_opt
{
- ListCell *li;
cypher_with *n;
/* check expressions are aliased */
- foreach(li, $3)
- {
- ResTarget *item = lfirst(li);
-
- /* variable does not have to be aliased */
- if (IsA(item->val, ColumnRef) || item->name)
- continue;
-
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("expression item must be aliased"),
- errhint("Items can be aliased by using AS."),
- ag_scanner_errposition(item->location, scanner)));
- }
+ validate_return_item_aliases($3, scanner);
n = make_ag_node(cypher_with);
n->distinct = true;
@@ -987,27 +922,12 @@ with:
$$ = (Node *)n;
}
- | WITH return_item_list order_by_opt skip_opt limit_opt
- where_opt
+ | WITH return_item_list order_by_opt skip_opt limit_opt where_opt
{
- ListCell *li;
cypher_with *n;
/* check expressions are aliased */
- foreach (li, $2)
- {
- ResTarget *item = lfirst(li);
-
- /* variable does not have to be aliased */
- if (IsA(item->val, ColumnRef) || item->name)
- continue;
-
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("expression item must be aliased"),
- errhint("Items can be aliased by using AS."),
- ag_scanner_errposition(item->location, scanner)));
- }
+ validate_return_item_aliases($2, scanner);
n = make_ag_node(cypher_with);
n->distinct = false;
@@ -2449,58 +2369,58 @@ qual_op:
*/
safe_keywords:
- ALL { $$ = pnstrdup($1, 3); }
- | ANALYZE { $$ = pnstrdup($1, 7); }
- | AND { $$ = pnstrdup($1, 3); }
- | AS { $$ = pnstrdup($1, 2); }
- | ASC { $$ = pnstrdup($1, 3); }
- | ASCENDING { $$ = pnstrdup($1, 9); }
- | BY { $$ = pnstrdup($1, 2); }
- | CALL { $$ = pnstrdup($1, 4); }
- | CASE { $$ = pnstrdup($1, 4); }
- | COALESCE { $$ = pnstrdup($1, 8); }
- | CONTAINS { $$ = pnstrdup($1, 8); }
- | COUNT { $$ = pnstrdup($1 ,5); }
- | CREATE { $$ = pnstrdup($1, 6); }
- | DELETE { $$ = pnstrdup($1, 6); }
- | DESC { $$ = pnstrdup($1, 4); }
- | DESCENDING { $$ = pnstrdup($1, 10); }
- | DETACH { $$ = pnstrdup($1, 6); }
- | DISTINCT { $$ = pnstrdup($1, 8); }
- | ELSE { $$ = pnstrdup($1, 4); }
- | ENDS { $$ = pnstrdup($1, 4); }
- | EXISTS { $$ = pnstrdup($1, 6); }
- | EXPLAIN { $$ = pnstrdup($1, 7); }
- | IN { $$ = pnstrdup($1, 2); }
- | IS { $$ = pnstrdup($1, 2); }
- | LIMIT { $$ = pnstrdup($1, 6); }
- | MATCH { $$ = pnstrdup($1, 6); }
- | MERGE { $$ = pnstrdup($1, 6); }
- | NOT { $$ = pnstrdup($1, 3); }
- | OPERATOR { $$ = pnstrdup($1, 8); }
- | OPTIONAL { $$ = pnstrdup($1, 8); }
- | OR { $$ = pnstrdup($1, 2); }
- | ORDER { $$ = pnstrdup($1, 5); }
- | REMOVE { $$ = pnstrdup($1, 6); }
- | RETURN { $$ = pnstrdup($1, 6); }
- | SET { $$ = pnstrdup($1, 3); }
- | SKIP { $$ = pnstrdup($1, 4); }
- | STARTS { $$ = pnstrdup($1, 6); }
- | THEN { $$ = pnstrdup($1, 4); }
- | UNION { $$ = pnstrdup($1, 5); }
- | WHEN { $$ = pnstrdup($1, 4); }
- | VERBOSE { $$ = pnstrdup($1, 7); }
- | WHERE { $$ = pnstrdup($1, 5); }
- | WITH { $$ = pnstrdup($1, 4); }
- | XOR { $$ = pnstrdup($1, 3); }
- | YIELD { $$ = pnstrdup($1, 5); }
+ ALL { $$ = KEYWORD_STRDUP($1); }
+ | ANALYZE { $$ = KEYWORD_STRDUP($1); }
+ | AND { $$ = KEYWORD_STRDUP($1); }
+ | AS { $$ = KEYWORD_STRDUP($1); }
+ | ASC { $$ = KEYWORD_STRDUP($1); }
+ | ASCENDING { $$ = KEYWORD_STRDUP($1); }
+ | BY { $$ = KEYWORD_STRDUP($1); }
+ | CALL { $$ = KEYWORD_STRDUP($1); }
+ | CASE { $$ = KEYWORD_STRDUP($1); }
+ | COALESCE { $$ = KEYWORD_STRDUP($1); }
+ | CONTAINS { $$ = KEYWORD_STRDUP($1); }
+ | COUNT { $$ = KEYWORD_STRDUP($1); }
+ | CREATE { $$ = KEYWORD_STRDUP($1); }
+ | DELETE { $$ = KEYWORD_STRDUP($1); }
+ | DESC { $$ = KEYWORD_STRDUP($1); }
+ | DESCENDING { $$ = KEYWORD_STRDUP($1); }
+ | DETACH { $$ = KEYWORD_STRDUP($1); }
+ | DISTINCT { $$ = KEYWORD_STRDUP($1); }
+ | ELSE { $$ = KEYWORD_STRDUP($1); }
+ | ENDS { $$ = KEYWORD_STRDUP($1); }
+ | EXISTS { $$ = KEYWORD_STRDUP($1); }
+ | EXPLAIN { $$ = KEYWORD_STRDUP($1); }
+ | IN { $$ = KEYWORD_STRDUP($1); }
+ | IS { $$ = KEYWORD_STRDUP($1); }
+ | LIMIT { $$ = KEYWORD_STRDUP($1); }
+ | MATCH { $$ = KEYWORD_STRDUP($1); }
+ | MERGE { $$ = KEYWORD_STRDUP($1); }
+ | NOT { $$ = KEYWORD_STRDUP($1); }
+ | OPERATOR { $$ = KEYWORD_STRDUP($1); }
+ | OPTIONAL { $$ = KEYWORD_STRDUP($1); }
+ | OR { $$ = KEYWORD_STRDUP($1); }
+ | ORDER { $$ = KEYWORD_STRDUP($1); }
+ | REMOVE { $$ = KEYWORD_STRDUP($1); }
+ | RETURN { $$ = KEYWORD_STRDUP($1); }
+ | SET { $$ = KEYWORD_STRDUP($1); }
+ | SKIP { $$ = KEYWORD_STRDUP($1); }
+ | STARTS { $$ = KEYWORD_STRDUP($1); }
+ | THEN { $$ = KEYWORD_STRDUP($1); }
+ | UNION { $$ = KEYWORD_STRDUP($1); }
+ | WHEN { $$ = KEYWORD_STRDUP($1); }
+ | VERBOSE { $$ = KEYWORD_STRDUP($1); }
+ | WHERE { $$ = KEYWORD_STRDUP($1); }
+ | WITH { $$ = KEYWORD_STRDUP($1); }
+ | XOR { $$ = KEYWORD_STRDUP($1); }
+ | YIELD { $$ = KEYWORD_STRDUP($1); }
;
conflicted_keywords:
- END_P { $$ = pnstrdup($1, 5); }
- | FALSE_P { $$ = pnstrdup($1, 7); }
- | NULL_P { $$ = pnstrdup($1, 6); }
- | TRUE_P { $$ = pnstrdup($1, 6); }
+ END_P { $$ = KEYWORD_STRDUP($1); }
+ | FALSE_P { $$ = KEYWORD_STRDUP($1); }
+ | NULL_P { $$ = KEYWORD_STRDUP($1); }
+ | TRUE_P { $$ = KEYWORD_STRDUP($1); }
;
%%
@@ -3384,7 +3304,7 @@ static Node *build_list_comprehension_node(Node *var,
Node *expr,
/*
* Build an ARRAY sublink and attach list_comp as sub-select,
- * it will be transformed in to query tree by us and reattached for
+ * it will be transformed in to query tree by us and reattached for
* pg to process.
*/
sub = makeNode(SubLink);
@@ -3396,3 +3316,60 @@ static Node *build_list_comprehension_node(Node *var,
Node *expr,
return (Node *) node_to_agtype((Node *)sub, "agtype[]", location);
}
+
+/* Helper function to create an ExplainStmt node */
+static ExplainStmt *make_explain_stmt(List *options)
+{
+ ExplainStmt *estmt = makeNode(ExplainStmt);
+ estmt->query = NULL;
+ estmt->options = options;
+ return estmt;
+}
+
+/* Helper function to validate that return items are properly aliased */
+static void validate_return_item_aliases(List *items, ag_scanner_t scanner)
+{
+ ListCell *li;
+
+ foreach(li, items)
+ {
+ ResTarget *item = lfirst(li);
+
+ /* variable does not have to be aliased */
+ if (IsA(item->val, ColumnRef) || item->name)
+ continue;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("expression item must be aliased"),
+ errhint("Items can be aliased by using AS."),
+ ag_scanner_errposition(item->location, scanner)));
+ }
+}
+
+/* Helper function to create a default return node (RETURN *) */
+static cypher_return *make_default_return_node(int location)
+{
+ ColumnRef *cr;
+ ResTarget *rt;
+ cypher_return *n;
+
+ cr = makeNode(ColumnRef);
+ cr->fields = list_make1(makeNode(A_Star));
+ cr->location = location;
+
+ rt = makeNode(ResTarget);
+ rt->name = NULL;
+ rt->indirection = NIL;
+ rt->val = (Node *)cr;
+ rt->location = location;
+
+ n = make_ag_node(cypher_return);
+ n->distinct = false;
+ n->items = list_make1((Node *)rt);
+ n->order_by = NULL;
+ n->skip = NULL;
+ n->limit = NULL;
+
+ return n;
+}
