fix(sql): allow aggregate functions inside CASE expressions by bluestreak01 · Pull Request #6550 · questdb/questdb

bluestreak01 · 2025-12-17T19:50:52Z

Summary

Queries with aggregate functions inside CASE expressions were failing with:

Aggregate function cannot be passed as an argument

For example, this query now works:

SELECT
  timestamp, symbol,
  first(price),
  last(price),
  CASE
    WHEN symbol = 'BTC-USD' THEN first(price)
    ELSE last(price)
  END
FROM trades
SAMPLE BY 1h

Root Cause

checkForChildAggregates() in SqlOptimiser only traversed lhs and rhs children of expression nodes. However, CASE expressions (and other functions with 3+ arguments) store their arguments in node.args, not lhs/rhs. This caused the optimizer to miss aggregate functions nested inside these expressions.

Fix

Modified checkForChildAggregates() to also iterate through node.args when node.paramCount >= 3, following the same pattern already used in emitAggregatesAndLiterals().

coderabbitai · 2025-12-17T19:50:58Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The PR refactors the aggregate detection traversal logic in SqlOptimiser.checkForChildAggregates() to handle multi-argument expressions like CASE statements. A new test validates SAMPLE BY queries with aggregates nested in CASE expressions.

Changes

Cohort / File(s)	Summary
Core logic refactoring `core/src/main/java/io/questdb/griffin/SqlOptimiser.java`	Expands aggregate detection traversal to iterate over multi-argument nodes (paramCount >= 3). Previously only inspected rhs for FUNCTION group-by nodes; now processes all arguments in CASE-like structures and pushes non-null arguments onto traversal stack for proper nested aggregate detection.
Test addition `core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java`	Adds `testCaseWithAggregatesInSampleBy()` method to verify SAMPLE BY queries with aggregate functions (`first()`, `last()`) nested inside CASE expressions execute correctly with ALIGN TO CALENDAR options.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

SqlOptimiser.java: The refactored traversal logic requires careful verification that the paramCount-based branching correctly handles all expression types and that stack-based traversal properly discovers nested aggregates in multi-argument nodes.
SampleByTest.java: The new test method validates the fix scenario but involves query result comparison with multiple sampling configurations.

Possibly related PRs

#6254: Modifies SqlOptimiser to alter sample-by and group-by rewrite logic, directly related to aggregate handling in SAMPLE BY contexts.

Suggested labels

Bug, SQL

Suggested reviewers

puzpuzpuz

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: allowing aggregate functions inside CASE expressions, which directly addresses the fix implemented in the code.
Description check	✅ Passed	The description is directly related to the changeset, explaining the problem, root cause, and solution with concrete examples and references to the modified method.
Linked Issues check	✅ Passed	The PR successfully addresses the primary objective of issue #6549 by enabling aggregate functions inside CASE expressions. The checkForChildAggregates() modification now properly traverses node.args for multi-argument functions, resolving the 'Aggregate function cannot be passed as an argument' error.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: the optimizer fix handles aggregates in CASE expressions, and the test verifies this specific scenario. No unrelated modifications are present.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

RaphDal · 2025-12-18T09:35:47Z

@coderabbitai review

coderabbitai · 2025-12-18T09:35:54Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java (1)

15947-15947: Consider keeping exception output on stderr.

The default printStackTrace() writes to System.err, which is the conventional stream for error output. Redirecting to System.out may cause exception traces to interleave with normal output in unexpected ways. If this change was intentional for specific test output capture needs, please disregard.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40367ef and bda4aed.

📒 Files selected for processing (2)

core/src/main/java/io/questdb/griffin/SqlOptimiser.java (1 hunks)
core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (35)

GitHub Check: New pull request (Coverage Report Coverage Report)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-other)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-pgwire)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-sub)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-cairo-root)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz2)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-fuzz1)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-sub)
GitHub Check: New pull request (Rust Test and Lint on linux-jdk17)
GitHub Check: New pull request (SelfHosted Running tests with cover on linux-griffin-root)
GitHub Check: New pull request (Hosted Running tests on windows-other-2)
GitHub Check: New pull request (Hosted Running tests on windows-other-1)
GitHub Check: New pull request (Hosted Running tests on windows-pgwire)
GitHub Check: New pull request (Hosted Running tests on windows-cairo-2)
GitHub Check: New pull request (Hosted Running tests on windows-cairo-1)
GitHub Check: New pull request (Hosted Running tests on windows-fuzz2)
GitHub Check: New pull request (Hosted Running tests on windows-fuzz1)
GitHub Check: New pull request (Hosted Running tests on windows-griffin-sub)
GitHub Check: New pull request (Hosted Running tests on windows-griffin-base)
GitHub Check: New pull request (Hosted Running tests on mac-other)
GitHub Check: New pull request (Hosted Running tests on mac-pgwire)
GitHub Check: New pull request (Hosted Running tests on mac-cairo-fuzz)
GitHub Check: New pull request (Hosted Running tests on mac-cairo)
GitHub Check: New pull request (Hosted Running tests on mac-griffin)
GitHub Check: New pull request (SelfHosted Other tests on linux-arm64)
GitHub Check: New pull request (SelfHosted Other tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Other tests on linux-x86-graal)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-x86-graal)
GitHub Check: New pull request (Trigger Enterprise CI Trigger Enterprise Pipeline)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-x64-zfs)
GitHub Check: New pull request (SelfHosted Griffin tests on linux-arm64)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-x86-graal)
GitHub Check: New pull request (SelfHosted Cairo tests on linux-arm64)
GitHub Check: New pull request (Check Changes Check changes)

🔇 Additional comments (4)

core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java (3)

160-196: LGTM! Well-structured regression test for issue #6549.

The test correctly validates that aggregate functions (first(), last()) work inside CASE expressions with SAMPLE BY. The data setup (8 records per symbol at 15-minute intervals) properly exercises hourly sampling, and the expected results correctly reflect the CASE logic: first(price) for BTC-USD and last(price) for ETH-USD.

201-206: Formatting improvement using text blocks.

237-242: Consistent text block formatting.

core/src/main/java/io/questdb/griffin/SqlOptimiser.java (1)

1217-1257: Aggregate detection now correctly traverses multi‑argument expressions (e.g., CASE) to find child group‑by functions

The revised checkForChildAggregates now:

Walks lhs/rhs for paramCount < 3 as before, and

Iterates node.args for paramCount >= 3, checking each arg for group‑by FUNCTIONs and pushing non‑null args onto sqlNodeStack for full traversal.

The loop invariant (node is only set to null when sqlNodeStack is empty) ensures all children—including aggregates nested inside CASE and other multi‑arg nodes—are visited. This aligns with how emitAggregatesAndLiterals traverses the tree and should resolve the missing‑aggregate issue described in the PR without introducing regressions.

glasstiger · 2025-12-18T11:42:59Z

[PR Coverage check]

😍 pass : 18 / 18 (100.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/griffin/SqlOptimiser.java	18	18	100.00%

fix(sql): allow aggregate functions inside CASE expressions

f303449

cleanup

bda4aed

coderabbitai bot reviewed Dec 18, 2025

View reviewed changes

Improve tests

0eb9c14

puzpuzpuz approved these changes Dec 18, 2025

View reviewed changes

puzpuzpuz added Enhancement Enhance existing functionality Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution and removed Enhancement Enhance existing functionality labels Dec 18, 2025

bluestreak01 merged commit 2f74209 into master Dec 18, 2025
4 of 6 checks passed

bluestreak01 deleted the vi_fix_cond_agg branch December 18, 2025 10:54

coderabbitai bot mentioned this pull request Jan 14, 2026

feat(sql): support nested window functions with deduplication optimization #6643

Merged

6 tasks

coderabbitai bot mentioned this pull request Jan 22, 2026

perf(sql): apply scan optimization for first/last/min/max(timestamp) as function argument #6690

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sql): allow aggregate functions inside CASE expressions#6550

fix(sql): allow aggregate functions inside CASE expressions#6550
bluestreak01 merged 3 commits intomasterfrom
vi_fix_cond_agg

bluestreak01 commented Dec 17, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 17, 2025 •

edited

Loading

Review skipped

Uh oh!

RaphDal commented Dec 18, 2025

Uh oh!

coderabbitai bot commented Dec 18, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

glasstiger commented Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

bluestreak01 commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Uh oh!

coderabbitai bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

RaphDal commented Dec 18, 2025

Uh oh!

coderabbitai bot commented Dec 18, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glasstiger commented Dec 18, 2025

[PR Coverage check]

file detail

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bluestreak01 commented Dec 17, 2025 •

edited

Loading

coderabbitai bot commented Dec 17, 2025 •

edited

Loading