Skip to content

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

Merged
bluestreak01 merged 1 commit intomasterfrom
optimize_rewrite_first
Jan 22, 2026
Merged

perf(sql): apply scan optimization for first/last/min/max(timestamp) as function argument #6690
bluestreak01 merged 1 commit intomasterfrom
optimize_rewrite_first

Conversation

@kafka1991
Copy link
Copy Markdown
Collaborator

@kafka1991 kafka1991 commented Jan 22, 2026 •
edited
Loading

Fixes #6633

Move rewriteSingleFirstLastGroupBy after rewriteSelectClause to optimise queries like SELECT dateadd('m', -15, max(ts)) FROM t where max aggregate function as function argument

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 22, 2026 •
edited
Loading

Walkthrough

This PR optimizes first/last/min/max timestamp functions when used as arguments to other functions (e.g., dateadd('m', -15, max(pickup_datetime))). It modifies the SQL optimizer to improve alias/token handling, limit-advice transfer, and GROUP BY rewrite logic for nested aggregate expressions.

Changes

Cohort / File(s) Summary
Core Optimizer Logic
core/src/main/java/io/questdb/griffin/SqlOptimiser.java		 Adds guard condition for GROUP BY rewrite path; introduces alias/token handling when converting windowed expressions with literal timestamp RHS mappings; transfers limit-advice to nested models; adjusts rewriteSingleFirstLastGroupBy invocation timing in optimization pipeline.
Model Update
core/src/main/java/io/questdb/griffin/model/QueryModel.java		 Adds public method replaceColumnNameMap() to update alias mappings and column-name mappings bidirectionally.
Test Coverage
core/src/test/java/io/questdb/test/griffin/SqlOptimiserTest.java		 Replaces testWindowFunctionDeduplicationWhenNestedInExpression with new comprehensive test testFirstLastTimestampFunctionAsArgument covering first/last/min/max timestamp functions nested in function arguments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #6643 — Modifies SqlOptimiser window-function handling and alias/literal propagation with related QueryModel changes.
  • #6414 — Modifies SqlOptimiser to preserve ordering semantics for first/last aggregates in GROUP BY contexts.
  • #6550 — Improves handling of aggregates nested inside expressions at the optimizer level.

Suggested labels

Enhancement, SQL

Suggested reviewers

  • puzpuzpuz
  • bluestreak01
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: applying scan optimization for first/last/min/max(timestamp) as function arguments, which aligns with the PR's objective to optimize aggregate-as-argument patterns.
Description check ✅ Passed The description is related to the changeset, identifying the linked issue and explaining the key change of moving rewriteSingleFirstLastGroupBy after rewriteSelectClause to optimize the specific query pattern.
Linked Issues check ✅ Passed The PR addresses the core requirement from issue #6633: enabling scan optimization for queries using aggregate functions (first/last/min/max) on timestamp columns as arguments to other functions like dateadd, matching the performance of non-aggregate equivalents.
Out of Scope Changes check ✅ Passed All changes are in-scope: reordering optimizer steps, adding alias/token handling for windowed expressions, adding a new replaceColumnNameMap method, and updating tests to validate the optimization for first/last/min/max timestamp aggregates as function arguments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)
core/src/main/java/io/questdb/griffin/SqlOptimiser.java

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.

@kafka1991 kafka1991 changed the title fix(sql): move rewriteSingleFirstLastGroupBy after rewriteSelectClause fix(sql): apply scan optimization for first/last/min/max(timestamp) as function argument Jan 22, 2026
@kafka1991 kafka1991 changed the title fix(sql): apply scan optimization for first/last/min/max(timestamp) as function argument perf(sql): apply scan optimization for first/last/min/max(timestamp) as function argument Jan 22, 2026
@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Jan 22, 2026

[PR Coverage check]

😍 pass : 24 / 25 (96.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/SqlOptimiser.java 20 21 95.24%
🔵 io/questdb/griffin/model/QueryModel.java 4 4 100.00%

@kafka1991 kafka1991 added the Performance Performance improvements label Jan 22, 2026
@kafka1991
Copy link
Copy Markdown
Collaborator Author

kafka1991 commented Jan 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 22, 2026

✅ 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.

@bluestreak01
Copy link
Copy Markdown
Member

bluestreak01 commented Jan 22, 2026

@CodeRabbit, what is the story?

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 22, 2026

Tip

For best results, initiate chat on the files or code changes.

@bluestreak01, here's the story:

The Problem

When users wrote queries like SELECT dateadd('m', -15, max(ts)) FROM table (using a timestamp aggregate as an argument to another function), the optimizer was performing a full GroupBy scan, which was significantly slower than equivalent forms.

The Inconsistency

Interestingly, these faster alternatives already worked:

  • Direct column usage: SELECT dateadd('m', -15, ts) FROM table → ⚡ fast
  • Derived table approach: SELECT dateadd('m', -15, max_ts) FROM (SELECT max(ts) as max_ts FROM table) → ⚡ fast
  • But aggregate-as-argument: SELECT dateadd('m', -15, max(ts)) FROM table → 🐌 slow (full scan)

The Fix

The PR reorders the optimizer passes - specifically moving rewriteSingleFirstLastGroupBy to run after rewriteSelectClause instead of before. This timing change allows the optimizer to recognize and apply the same scan optimization to the aggregate-as-argument pattern that was already working for the other forms.

The Impact

Queries using first/last/min/max(timestamp) as function arguments now get the same optimized execution plan as the workaround patterns, eliminating the need for users to restructure their queries with derived tables.

The coverage looks solid at 96%, and the change is focused on the optimizer control flow in SqlOptimiser.java with supporting alias-mapping logic in QueryModel.java.

@bluestreak01 bluestreak01 merged commit b86cc42 into master Jan 22, 2026
46 checks passed
@bluestreak01 bluestreak01 deleted the optimize_rewrite_first branch January 22, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluestreak01 bluestreak01 bluestreak01 approved these changes

Assignees

No one assigned

Labels

Performance Performance improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Potential performance optimization max(timestamp) as a function arg

3 participants

@kafka1991 @glasstiger @bluestreak01