fix(sql): fix reverse-order argument parsing in multi-arg window function by mtopolnik · Pull Request #6697 · questdb/questdb

mtopolnik · 2026-01-23T12:23:40Z

coderabbitai · 2026-01-23T12:23:47Z

Walkthrough

Fixed argument collection order in WindowExprTreeBuilder (ExpressionParser) so multi-argument and nested window functions receive args in forward order; added and reorganized many tests covering nested window functions, CASE interactions, ORDER/PARTITION BY arity, and multi-argument functions.

Changes

Cohort / File(s)	Summary
Window expression argument ordering fix `core/src/main/java/io/questdb/griffin/ExpressionParser.java`	Adjusted WindowExprTreeBuilder to pop/collect function arguments in forward order (i = 0..paramCount-1) instead of reverse, changing observed argument ordering for multi-arg and nested window calls.
Window function unit tests (engine/window) `core/src/test/java/io/questdb/test/griffin/engine/window/WindowFunctionTest.java`	Added many new tests for nested window functions, column alias resolution, CASE WHEN interaction, ORDER BY column references, and multi-argument functions in PARTITION BY (e.g., `split_part`).
Expression parser tests (window-related) `core/src/test/java/io/questdb/test/griffin/ExpressionParserTest.java`	Replaced/added tests around nested window-as-argument cases, CASE with window functions, ORDER BY framing errors, and 3-arg/arity edge cases; reorganized existing window-related test coverage.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

feat(sql): implement arithmetic with window functions #6626 — touches WindowExprTreeBuilder/window-over parsing and relates to argument collection behavior.
feat(sql): support nested window functions with deduplication optimization #6643 — modifies ExpressionParser for nested window support; complements the arg-ordering fix.

Suggested labels

Bug, SQL

Suggested reviewers

puzpuzpuz

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.54% 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 fix: correcting reverse-order argument parsing for multi-argument window functions, which aligns with the primary code change in ExpressionParser.java.
Description check	✅ Passed	The description references issue `#6695`, which documents the type inference regression caused by improper argument ordering in window functions—directly related to the changeset.
Linked Issues check	✅ Passed	The code changes fix the argument ordering issue in ExpressionParser.java by collecting arguments in forward order instead of reverse order, directly addressing issue `#6695`'s requirement to restore correct argument ordering for multi-argument functions in window partition expressions.
Out of Scope Changes check	✅ Passed	All changes are in-scope: ExpressionParser.java fixes the core argument-ordering bug, and test additions in WindowFunctionTest.java and ExpressionParserTest.java validate the fix and nested window function scenarios related to the issue.

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

✨ Finishing touches

📝 Generate docstrings

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

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

mtopolnik · 2026-01-23T12:31:31Z

@CodeRabbit review this PR, please.

coderabbitai · 2026-01-23T12:31:47Z

@mtopolnik I'll review this PR for you right away.

🧠 Learnings used

Learnt from: bluestreak01
Repo: questdb/questdb PR: 6643
File: core/src/main/java/io/questdb/griffin/SqlOptimiser.java:8671-8687
Timestamp: 2026-01-20T00:59:19.630Z
Learning: QuestDB’s parser rejects pure window functions (e.g., row_number(), rank()) used without an OVER clause; such invocations are invalid and fail early, so validators don’t need to catch pure-name window functions in window specs.

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

glasstiger · 2026-01-23T13:36:03Z

[PR Coverage check]

😍 pass : 2 / 2 (100.00%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/griffin/ExpressionParser.java	2	2	100.00%

bluestreak01 · 2026-01-23T14:22:15Z

@CodeRabbit what is the story with your review?

coderabbitai · 2026-01-23T14:22:35Z

@bluestreak01 I apologize for the delay. Let me complete the review of this PR now.

🧠 Learnings used

Learnt from: bluestreak01
Repo: questdb/questdb PR: 6643
File: core/src/main/java/io/questdb/griffin/SqlOptimiser.java:8671-8687
Timestamp: 2026-01-20T00:59:19.630Z
Learning: QuestDB’s parser rejects pure window functions (e.g., row_number(), rank()) used without an OVER clause; such invocations are invalid and fail early, so validators don’t need to catch pure-name window functions in window specs.

Learnt from: bluestreak01
Repo: questdb/questdb PR: 6673
File: core/src/test/java/io/questdb/test/griffin/WindowJoinTest.java:2616-2618
Timestamp: 2026-01-19T21:43:19.228Z
Learning: In test `testWindowJoinFailsWhenSlaveDoesNotSupportTimeFrames` in `core/src/test/java/io/questdb/test/griffin/WindowJoinTest.java`, the use of `AND` between timestamp equality conditions (e.g., `ts = 'X' AND ts = 'Y'`) is intentional to prevent intrinsic interval creation. This ensures the window join fails as expected when the slave doesn't support time frames, which is necessary after PR `#6673` added OR timestamp intrinsics recognition.

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

mtopolnik added 3 commits January 23, 2026 13:22

Auto-reorder test methods

0e4f54b

Failing test for multi-arg window function

390acd9

Fix for arg order in window function

bf72f86

mtopolnik added 2 commits January 23, 2026 14:02

Fix arg order in ExpressionParserTest

a2d45ec

Auto-reorder test methods

45fcc62

bluestreak01 approved these changes Jan 23, 2026

View reviewed changes

bluestreak01 merged commit 9a52e20 into master Jan 23, 2026
43 checks passed

bluestreak01 deleted the mt_type-inference branch January 23, 2026 18:54

coderabbitai bot mentioned this pull request Feb 9, 2026

feat(sql): window definition #6746

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sql): fix reverse-order argument parsing in multi-arg window function#6697

fix(sql): fix reverse-order argument parsing in multi-arg window function#6697
bluestreak01 merged 5 commits intomasterfrom
mt_type-inference

mtopolnik commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026 •

edited

Loading

Review ran into problems

Uh oh!

mtopolnik commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026

Uh oh!

glasstiger commented Jan 23, 2026

Uh oh!

bluestreak01 commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mtopolnik commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Review ran into problems

Uh oh!

mtopolnik commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026

Uh oh!

glasstiger commented Jan 23, 2026

[PR Coverage check]

file detail

Uh oh!

bluestreak01 commented Jan 23, 2026

Uh oh!

coderabbitai bot commented Jan 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Jan 23, 2026 •

edited

Loading