#3761 fix: Cypher WITH *, extra AS alias dropped extra items#3762
#3761 fix: Cypher WITH *, extra AS alias dropped extra items#3762
Conversation
CypherASTBuilder.visitWithClause used an if/else so when TIMES (*) was present the returnItem() loop never ran — additional aliases were silently dropped. The RETURN clause then referenced an undefined alias, producing "Error parsing OpenCypher query". Fix: - CypherASTBuilder: changed if/else to if + unconditional loop so both the wildcard and any explicit items are always appended. - WithStep.projectResult: unified loop handles * (copy all input props) and explicit items (evaluate expression) in a single pass. - CypherSemanticValidator: when hasWildcard, still validate and register extra aliases so they are in scope for subsequent clauses. Regression test: Issue3761Test (5 cases covering the exact issue query, WITH * alone, computed alias, multiple aliases, alias in WHERE). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 7 |
🟢 Coverage 95.65% diff coverage · -8.42% coverage variation
Metric Results Coverage variation ✅ -8.42% coverage variation Diff coverage ✅ 95.65% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (4b31312) 115697 85582 73.97% Head commit (06f00d6) 147055 (+31358) 96389 (+10807) 65.55% (-8.42%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3762) 23 22 95.65% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Code Review
This pull request fixes a bug in OpenCypher where using a wildcard in a WITH clause followed by an alias (e.g., WITH *, expr AS alias) resulted in the alias being dropped or causing parsing errors. The changes involve updating the AST builder to process all return items regardless of the wildcard, refactoring the execution step to handle both wildcard projections and explicit expressions, and adjusting the semantic validator to account for these extra aliases. A comprehensive regression test suite was also added. Feedback was provided regarding the semantic validator, specifically pointing out that aliases should not be visible within the same WITH clause and that the current implementation incorrectly skips subsequent validation logic like ORDER BY when a wildcard is present.
| for (final ReturnClause.ReturnItem item : withClause.getItems()) { | ||
| if (item.getExpression() instanceof StarExpression || | ||
| (item.getExpression() instanceof VariableExpression && | ||
| "*".equals(((VariableExpression) item.getExpression()).getVariableName()))) | ||
| continue; | ||
| checkExpressionScope(item.getExpression(), scope); | ||
| final String alias = item.getAlias(); | ||
| if (alias != null) | ||
| scope.add(alias); | ||
| } | ||
| break; |
There was a problem hiding this comment.
The implementation of the hasWildcard block contains two significant issues:
-
Scope Leakage: By calling
scope.add(alias)(line 395) inside the same loop that performscheckExpressionScope(line 392), you are making aliases available to subsequent items in the sameWITHclause. In Cypher, an alias defined in aWITHclause should only be visible to the following clauses, not to other expressions within the sameWITHclause. -
Validation Bypass: The
breakstatement at line 397 causes the validator to skip theORDER BYvalidation logic (lines 403-433) and other subsequent checks for theWITHclause when a wildcard is present. This means queries likeWITH * ORDER BY undefined_varwould incorrectly pass semantic validation.
To fix this, consider validating all expressions against the incoming scope first, then performing ORDER BY validation, and finally updating the scope for the next clause.
There was a problem hiding this comment.
Scope leakage is a valid catch - fixed in 06f00d6 by splitting the block into two separate loops: validate all expressions first against the incoming scope, then add aliases. This matches the pattern of the non-wildcard path.
On ORDER BY bypass: the break was already there before this PR (the original code was simply break with no validation at all for WITH *). That's a pre-existing gap, not a regression introduced here.
In the hasWildcard block of CypherSemanticValidator, the previous implementation added aliases to scope inside the same loop that validated expressions. This allowed a later item's expression to reference an alias from an earlier item in the same WITH clause, which is incorrect Cypher semantics. Fix: two separate loops — validate all expressions first against the incoming scope, then add extra aliases for subsequent clauses. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3762 +/- ##
==========================================
+ Coverage 65.10% 65.15% +0.05%
==========================================
Files 1580 1580
Lines 115697 115940 +243
Branches 24517 24587 +70
==========================================
+ Hits 75329 75546 +217
+ Misses 30142 30121 -21
- Partials 10226 10273 +47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Fixes #3761 —
WITH *, expr AS aliasthrew "Error parsing OpenCypher query" because the alias was silently dropped during AST construction.Root causes (three locations):
CypherASTBuilder.visitWithClause: usedif/else— whenTIMES (*)was present, thereturnItem()loop was skipped, soWITH *, t2 AS out2produced items[*]only.WithStep.projectResult: checkeditems.size() == 1 && name == "*"and returned early, so a mixed list never evaluated the non-wildcard items.CypherSemanticValidator: whenhasWildcardwas true, broke out immediately without registering extra aliases in scope — causing the "undefined variable" error on the RETURN clause.Fixes:
CypherASTBuilder: changedif/elsetoif+ unconditional loop so both the wildcard and any explicit items are always appended.WithStep.projectResult: unified loop —*copies all input properties, explicit items evaluate the expression.CypherSemanticValidator: when wildcard is present, still validate and register extra aliases in scope.Test plan
Issue3761Test(5 cases): exact issue query,WITH *alone, computed alias, multiple aliases, alias in WHEREWithAndUnwindTest,OpenCypherOptionalMatchTest,OpenCypherExecutionTest,OpenCypherBasicTest,CypherTest,Issue3758Test— all green🤖 Generated with Claude Code