Skip to content

fix(sql): fix WINDOW JOIN INCLUDE PREVAILING dropping prevailing row when window has matches#6868

Merged
bluestreak01 merged 1 commit intomasterfrom
rd_fix_window_join_prevailing
Mar 11, 2026
Merged

fix(sql): fix WINDOW JOIN INCLUDE PREVAILING dropping prevailing row when window has matches#6868
bluestreak01 merged 1 commit intomasterfrom
rd_fix_window_join_prevailing

Conversation

@RaphDal
Copy link
Copy Markdown
Contributor

@RaphDal RaphDal commented Mar 11, 2026

Summary

WINDOW JOIN with INCLUDE PREVAILING includes right-table rows within the time window plus the most recent right row with a timestamp equal to or earlier than the window start.

The non-vectorized fast cursor (WindowJoinWithPrevailingFastRecordCursor) compared the first window match against the window end instead of the window start, making the prevailing row effectively never included when the window already contained matches:

// bug: masterTimestampHi is the window end — since window rows always
// have ts <= windowEnd, this condition is never true
if (rowLo >= rowHi || slaveTimestamps.get(rowLo) > masterTimestampHi)

// fix: slaveTimestampLo is the window start, consistent with the
// vectorized cursor
if (rowLo >= rowHi || slaveTimestamps.get(rowLo) > slaveTimestampLo)

The vectorized variant (WindowJoinFastVectRecordCursor) already used the correct comparison. This bug only affected queries using non-vectorizable aggregates (e.g., max(concat(...))) with symbol-keyed WINDOW JOIN and INCLUDE PREVAILING.

Test plan

testFastNonVectorizedPrevailingWithWindowMatches uses max(concat(r.val, '')) to force the non-vectorized fast path with a prevailing row (val=99) outside the window alongside two window matches (val=20, 30).
Expects max="99"; without the fix, returns "30".
Cross-checks with vectorized count(*) expecting 3 rows.

@RaphDal RaphDal requested a review from puzpuzpuz March 11, 2026 15:45
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 11, 2026

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.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9d96fc10-1e56-402e-887c-bfc4a431b60c

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rd_fix_window_join_prevailing

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.

@glasstiger
Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 4 / 4 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/join/WindowJoinTimeFrameHelper.java 3 3 100.00%
🔵 io/questdb/griffin/engine/join/WindowJoinFastRecordCursorFactory.java 1 1 100.00%

@bluestreak01 bluestreak01 merged commit 93279ae into master Mar 11, 2026
51 checks passed
@bluestreak01 bluestreak01 deleted the rd_fix_window_join_prevailing branch March 11, 2026 18:53
mtopolnik pushed a commit that referenced this pull request Mar 12, 2026
maciulis pushed a commit to maciulis/questdb that referenced this pull request Mar 16, 2026
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.

4 participants