feat(parquet): separate push decoder frontier state from row-group decoding#9804
Open
HippoBaro wants to merge 3 commits intoapache:mainfrom
Open
feat(parquet): separate push decoder frontier state from row-group decoding#9804HippoBaro wants to merge 3 commits intoapache:mainfrom
HippoBaro wants to merge 3 commits intoapache:mainfrom
Conversation
Extract the push decoder offset/limit accounting into `RowBudget` and use it when planning row-group reads. This centralizes the row-count arithmetic needed to apply offset and limit without changing decoder behavior. It also adds focused tests for plain limit, offset+limit, and empty-selection cases so later frontier work can reuse the same accounting safely. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
Move the cross-row-group scan state into a dedicated `RowGroupFrontier`. The frontier now owns the queued row groups, the tail `RowSelection`, the running `RowBudget`, and the conservative "has predicates" flag. Reduce `RowGroupReaderBuilder` to current-row-group work only by threading a budget snapshot into `next_row_group` and returning a typed `RowGroupBuildResult`. This also folds in the selection-frontier cleanup so queued selection state is consumed in one place instead of through ad hoc split/clone logic. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
Teach the row-group frontier to seek ahead over queued row groups that can be proven unreachable before instantiating the row-group builder. Skip queued row groups when their selection slice is empty, when offset/limit leaves no rows to read, or when the remaining limit is already exhausted. Keep predicate-bearing row groups conservative and stop at the first row group that may still need data. Add a push decoder regression covering `try_next_reader` with offset/limit so the frontier path is exercised directly. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
HippoBaro
force-pushed
the
frontier_row_group_selection
branch
from
600500d to
307e4d8
Compare
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
1 participant
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.
Which issue does this PR close?
PushBuffersboundary-agnostic for prefetch IO #9697Rationale for this change
#9697 aims to make staged buffer management in the push decoder more explicit. In doing so, it exposes a structural problem: the logic for deciding whether a row group is still live, skipped, or unreachable is spread across several parts of the decoder.
This matters because row-group-level buffer release depends on a single question having a clear answer: can this row group ever need bytes again? That answer depends on the queued row groups, the remaining selection, the running offset/limit budget, and whether predicates require the decoder to stay conservative. Today, that state is split across multiple components, which makes the release policy difficult to centralize cleanly.
What changes are included in this PR?
This PR introduces a clearer ownership boundary in the push decoder:
This does not implement row-group-level buffer release directly, but it establishes the structure needed for that follow-up work. It should also make future pruning rules easier to add and maintain.
Are these changes tested?
All existing tests pass, and the refactor adds focused coverage for the extracted budget logic and the frontier-driven
try_next_readerpath.Are there any user-facing changes?
None.