Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,22 @@ Expression parseExpression(final Cypher25Parser.ExpressionContext ctx) {
* This is a helper for parsing lower-level expression contexts.
*/
Expression parseExpressionFromText(final ParseTree node) {
// Check for CASE expressions in the parse tree
// All recursive searches below use a length guard: only match when the found
// context covers (almost) the full node text, preventing mis-parsing of
// func(CASE ...) as just the inner CASE. The - 2 allows for whitespace.
final String nodeText = node.getText();

final Cypher25Parser.CaseExpressionContext caseCtx = findCaseExpressionRecursive(node);
if (caseCtx != null)
if (caseCtx != null && caseCtx.getText().length() >= nodeText.length() - 2)
return parseCaseExpression(caseCtx);

final Cypher25Parser.ExtendedCaseExpressionContext extCaseCtx = findExtendedCaseExpressionRecursive(node);
if (extCaseCtx != null)
if (extCaseCtx != null && extCaseCtx.getText().length() >= nodeText.length() - 2)
return parseExtendedCaseExpression(extCaseCtx);
Comment on lines +248 to 256
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The node.getText() call is performed eagerly, which is inefficient if no CASE expression is found in the subtree. More importantly, the recursive search logic used here is also applied to EXISTS (line 258) and shortestPath expressions, but they lack the length guard. This means expressions like count(EXISTS { ... }) or my_func(shortestPath(...)) will still be mis-parsed as just the inner special expression, losing the outer wrapper and potentially causing incorrect query results. You should apply the same guard to all special expressions that use recursive search to ensure consistent and correct parsing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied the length guard to EXISTS in both tryParseSpecialFunctions and parseExpressionFromText, and to shortestPath in tryParseSpecialFunctions. Also consolidated the exprText/nodeText variable to the top with an inline comment explaining the - 2 tolerance. All 5147 Cypher tests still pass.


// Check for EXISTS expressions
final Cypher25Parser.ExistsExpressionContext existsCtx = findExistsExpressionRecursive(node);
if (existsCtx != null)
if (existsCtx != null && existsCtx.getText().length() >= nodeText.length() - 2)
return parseExistsExpression(existsCtx);

// Check for logical expressions (AND, OR, XOR, NOT) in the parse tree
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,30 @@ Expression tryParseSpecialFunctions(final Cypher25Parser.ExpressionContext ctx)
return new FunctionCallExpression("count", args, false);
}

// All recursive searches below use a length guard: only match when the found
// context covers (almost) the full expression text. Without this, expressions
// like sum(CASE WHEN ... END) would be mis-parsed as just the inner special
// expression, losing the outer function wrapper. The - 2 tolerance allows for
// whitespace that ANTLR's getText() strips from tokens.
final String exprText = ctx.getText();

// EXISTS expression
final Cypher25Parser.ExistsExpressionContext existsCtx = builder.findExistsExpressionRecursive(ctx);
if (existsCtx != null)
if (existsCtx != null && existsCtx.getText().length() >= exprText.length() - 2)
return builder.parseExistsExpression(existsCtx);

// CASE expressions (both forms)
final Cypher25Parser.CaseExpressionContext caseCtx = builder.findCaseExpressionRecursive(ctx);
if (caseCtx != null)
if (caseCtx != null && caseCtx.getText().length() >= exprText.length() - 2)
return builder.parseCaseExpression(caseCtx);

final Cypher25Parser.ExtendedCaseExpressionContext extCaseCtx = builder.findExtendedCaseExpressionRecursive(ctx);
if (extCaseCtx != null)
if (extCaseCtx != null && extCaseCtx.getText().length() >= exprText.length() - 2)
return builder.parseExtendedCaseExpression(extCaseCtx);

// shortestPath expressions
final Cypher25Parser.ShortestPathExpressionContext shortestPathCtx = builder.findShortestPathExpressionRecursive(ctx);
if (shortestPathCtx != null)
if (shortestPathCtx != null && shortestPathCtx.getText().length() >= exprText.length() - 2)
return builder.parseShortestPathExpression(shortestPathCtx);

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,52 @@ void caseWithNullHandling() {
assertThat((Object) result.getProperty("category")).isEqualTo("unknown");
}

/**
* Regression test for issue #3858:
* Aggregation with CASE statement should not create implicit GROUP BY on variables inside the CASE.
* When all RETURN items are aggregations, a single row must be returned.
*/
@Test
void aggregationWithCaseNoImplicitGroupBy() {
// Both count(loc) and sum(CASE...) are aggregations — no grouping keys exist.
// Expected: single row {total: 3, pa: 2}
final ResultSet results = database.query("opencypher",
"UNWIND [{state: 'NY'}, {state: 'PA'}, {state: 'PA'}] AS loc " +
"RETURN count(loc) AS total, sum(CASE WHEN loc.state = 'PA' THEN 1 ELSE 0 END) AS pa");

assertThat((boolean) results.hasNext()).isTrue();
final Result result = results.next();
assertThat(((Number) result.getProperty("total")).longValue()).isEqualTo(3L);
assertThat(((Number) result.getProperty("pa")).longValue()).isEqualTo(2L);
assertThat((boolean) results.hasNext()).isFalse();
}

/**
* Regression test for issue #3858 (variant):
* When RETURN has both aggregations and a non-aggregated grouping key,
* the CASE arguments must not act as additional grouping keys.
*/
@Test
void aggregationWithCaseAndGroupByKey() {
// category is the real grouping key; sum(CASE...) must aggregate per category, not per loc.state
final ResultSet results = database.query("opencypher",
"UNWIND [{state: 'NY', cat: 'A'}, {state: 'PA', cat: 'A'}, {state: 'PA', cat: 'B'}] AS loc " +
"RETURN loc.cat AS category, sum(CASE WHEN loc.state = 'PA' THEN 1 ELSE 0 END) AS pa " +
"ORDER BY category");

assertThat((boolean) results.hasNext()).isTrue();
final Result rowA = results.next();
assertThat((Object) rowA.getProperty("category")).isEqualTo("A");
assertThat(((Number) rowA.getProperty("pa")).longValue()).isEqualTo(1L);

assertThat((boolean) results.hasNext()).isTrue();
final Result rowB = results.next();
assertThat((Object) rowB.getProperty("category")).isEqualTo("B");
assertThat(((Number) rowB.getProperty("pa")).longValue()).isEqualTo(1L);

assertThat((boolean) results.hasNext()).isFalse();
}

@Test
void nestedCaseExpressions() {
// Nested CASE expressions
Expand Down
Loading