fix(sql): WINDOW JOIN dropping prevailing row on cross-partition#6871
Merged
bluestreak01 merged 5 commits intomasterfrom Mar 13, 2026
Merged
fix(sql): WINDOW JOIN dropping prevailing row on cross-partition#6871bluestreak01 merged 5 commits intomasterfrom
bluestreak01 merged 5 commits intomasterfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
puzpuzpuz
reviewed
Mar 12, 2026
core/src/main/java/io/questdb/griffin/engine/join/WindowJoinTimeFrameHelper.java
Outdated
Show resolved
Hide resolved
…ss-partition scenarios
puzpuzpuz
added
Bug
puzpuzpuz
changed the title
puzpuzpuz
approved these changes
Mar 13, 2026
Contributor
[PR Coverage check]😍 pass : 0 / 0 (0%) |
maciulis
pushed a commit
to maciulis/questdb
that referenced
this pull request
Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WindowJoinTimeFrameHelper.findRowLo()drops the INCLUDE PREVAILING row when the bookmarked-frame optimization kicks in and the prevailing candidate sits in a prior partition. The bookmarked path skipped initialization ofprevailingFrameIndex/prevailingRowIndex, leaving them at-1/Long.MIN_VALUE. Downstream backward scans (findPrevailingForMasterRow,WindowJoinPrevailingCache) saw-1and returned immediately, silently omitting the prevailing row from results.WindowJoinWithPrevailingFastRecordCursor.hasNext()comparedslaveTimestamps.get(rowLo)againstmasterTimestampHiinstead ofslaveTimestampLowhen deciding whether to include the prevailing value. This caused it to skip prevailing inclusion when the first in-index row fell betweenslaveTimestampLoandmasterTimestampHi.Detail
The first bug affects all four sync cursor variants that call
findRowLo(lo, hi, true):WindowJoinWithPrevailingAndJoinFilterRecordCursorWindowJoinWithPrevailingAndJoinFilterFastRecordCursorWindowJoinWithPrevailingFastRecordCursorWindowJoinFastVectRecordCursor(whenincludePrevailing=true)The base
WindowJoinRecordCursor(prevailing without filter) is not affected because it uses the separatefindRowLoWithPrevailing()method, which maintains its own local prevailing tracking and handles the bookmarked path correctly.The fix seeds
prevailingFrameIndex/prevailingRowIndexwhen entering via the bookmark path. If the bookmark is past the frame start, the row immediately before the bookmark becomes the prevailing candidate. If the bookmark is at the frame start, the prevailing index is set torowLo - 1, which signalsfindPrevailingForMasterRowto skip to the previous frame.The second bug is a simple comparison target error in
WindowJoinWithPrevailingFastRecordCursor: the condition that triggers prevailing inclusion should check whether the first row's timestamp exceeds the window low boundary (slaveTimestampLo), not the window high boundary (masterTimestampHi).Test plan
testPrevailingWithFilterCrossPartitionregression test toWindowJoinTest: master rows on day 2, slave rows on day 1 and day 2, join filter onidcolumn, 1-second window with INCLUDE PREVAILING. Verifies the cross-partition prevailing row is counted. Fails without the fix, passes with it.