Skip to content

[SQL] Generalize optimization rule for MAX(CASE WHEN expression THEN 1 END)#6591

Merged
mihaibudiu merged 1 commit into
feldera:mainfrom
mihaibudiu:issue5690
Jul 6, 2026
Merged

[SQL] Generalize optimization rule for MAX(CASE WHEN expression THEN 1 END)#6591
mihaibudiu merged 1 commit into
feldera:mainfrom
mihaibudiu:issue5690

Conversation

@mihaibudiu

@mihaibudiu mihaibudiu commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #6590

Checklist

  • Unit tests added/updated

@mihaibudiu
mihaibudiu requested a review from anandbraman July 3, 2026 19:11
…1 END)

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Generalization is sound.

Semantics (ELSE NULL / no ELSE): With GROUP BY, every materialized group has ≥1 row, so MAX-of-empty-group isn't reachable. Per-row values are 1 (P true) or NULL (P false / P NULL). MAX ignores NULLs, so the result is 1 iff any row has P true, else NULL. The rewrite CASE WHEN COUNT_FILTER(P IS TRUE) > 0 THEN 1 ELSE NULL END matches: count > 0 iff a row has P true. IS TRUE correctly folds NULL predicates into the "no match" bucket. COUNT never returns NULL, so no defensive branch needed for that side.

Nice bonus: the drive-by fix to the post-project input-ref index (groupCount + newAggregates.size() instead of just newAggregates.size()) corrects a latent bug for untransformed aggregate calls in mixed aggregates. issue6590b (MAX + SUM) exercises exactly that path.

Tests: cover empty group, P-true, P-false, P-NULL, and the mixed-aggregate index path. Matches Postgres.

One tiny nit inline — non-blocking.

return null;
Long op2Value = getLiteralValue(op2);
if (op2Value == null)
if (op2Value == null && !(op2 instanceof RexLiteral))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: op2Value == null && !(op2 instanceof RexLiteral) reads as "reject non-literal op2", but you already know getLiteralValue returned null. A more direct spelling would be op2 instanceof RexLiteral lit && lit.isNull() (accept only null literal). Same behavior, clearer intent. Non-blocking.

@mihaibudiu
mihaibudiu added this pull request to the merge queue Jul 6, 2026
Merged via the queue into feldera:main with commit 914012b Jul 6, 2026
1 check passed
@mihaibudiu
mihaibudiu deleted the issue5690 branch July 6, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SQL] Optimize MAX(CASE WHEN expression THEN 1 END)

3 participants