Copilot commented on code in PR #2361:
URL: https://github.com/apache/age/pull/2361#discussion_r2991777677
##########
src/backend/parser/cypher_gram.y:
##########
@@ -3318,6 +3329,30 @@ static Node *build_list_comprehension_node(Node *var,
Node *expr,
}
/* Helper function to create an ExplainStmt node */
+/*
+ * Wrap a graph pattern in an EXISTS SubLink. Used by both
+ * EXISTS(pattern) syntax and bare pattern expressions in WHERE.
+ */
+static Node *make_exists_pattern_sublink(Node *pattern, int location)
Review Comment:
The comment "Helper function to create an ExplainStmt node" now sits
immediately above make_exists_pattern_sublink(), which is unrelated to EXPLAIN.
This can confuse future readers; please move the ExplainStmt comment down to
just before make_explain_stmt(), or update/remove it so it matches the function
that follows.
##########
Makefile:
##########
@@ -111,8 +111,10 @@ REGRESS = scan \
name_validation \
jsonb_operators \
list_comprehension \
+ pattern_expression \
map_projection \
direct_field_access \
+ agtype_jsonb_cast \
security
Review Comment:
The PR description/title focus on agtype↔jsonb casts, but this Makefile
change adds new regression suites for pattern expressions (and the PR also
modifies the Cypher grammar). Please update the PR description to reflect the
parser/pattern-expression work or split the changes into separate PRs so
reviewers can evaluate scope and risk appropriately.
##########
src/backend/parser/cypher_gram.y:
##########
@@ -49,6 +49,17 @@
%locations
%name-prefix="cypher_yy"
%pure-parser
+/*
+ * GLR mode handles the ambiguity between parenthesized expressions and
+ * graph patterns. For example, WHERE (a)-[:KNOWS]->(b) starts with (a)
+ * which is valid as both an expression and a path_node. The parser forks
+ * at the conflict point and discards the failing path. %dprec annotations
+ * on expr_var/var_name_opt and '(' expr ')'/anonymous_path resolve cases
+ * where both paths succeed (bare (a) prefers the expression interpretation).
+ */
+%glr-parser
+%expect 7
+%expect-rr 3
Review Comment:
Using %expect/%expect-rr makes the build fail if Bison reports a different
conflict count (which can vary across Bison versions). Since CI/tooling doesn’t
appear to pin Bison, this can introduce fragile, version-dependent build
failures; consider resolving the conflicts explicitly (so %expect can be
removed) or pinning/validating the Bison version used to generate the parser.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
