feat(nimble): Deserializer honors kTablet per-batch rowRange (#1045) - #1045
Closed
tanjialiang wants to merge 1 commit into
Closed
feat(nimble): Deserializer honors kTablet per-batch rowRange (#1045)#1045tanjialiang wants to merge 1 commit into
tanjialiang wants to merge 1 commit into
Conversation
|
@tanjialiang has exported this pull request. If you are a Meta employee, you can view the originating Diff in D113966951. |
tanjialiang
force-pushed
the
export-D113966951
branch
from
July 30, 2026 19:38
f73eadc to
32227aa
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Jul 30, 2026
…kincubator#1045) Summary: Flips `nimble::Deserializer` behavior on kTablet-format batches: the per-batch `rowRange` embedded in each kTablet chunk header is now honored end-to-end, and the decoded output contains only the requested row window. Previously the Deserializer over-fetched — it read the `rowRange` field off the wire (as parsed by `serde::StreamDataParser`) but decoded the full stripe and left slicing to the caller. ## Design: slice at ingest, decode as usual `appendBatch` pulls `parser_->rowRange()` and forwards it to `appendStreamSegments`, which takes a `nimble::RowRange`. When `rowRange.numRows() != parser_->rowCount()` each per-stream encoded blob is trimmed via `EncodingSliceFactory::slice` before being queued to its decoder. The decoder path itself is unchanged — one `reader_->next(rowCount, out)` reads the already-trimmed segments. When the rowRange covers the full batch (the case for non-kTablet batches, which get a default `{0, rowCount}` from `appendBatch`) the fast path (no slicing) fires. ## Value streams need in-map-aware translation FlatMap value streams are indexed by the compact set-bit domain of the sibling in-map, not the top-level row domain, so a top-level rowRange does not translate directly to value-stream offsets. `appendStreamSegments` runs a two-pass loop: 1. **Pass 1** (inside `iterateStreams`): in-map and scalar streams slice directly (1:1 with top-level rows). For in-map streams, materialize the original unsliced bitmap into a per-offset cache. Defer value streams to pass 2. 2. **Pass 2** (after `iterateStreams`): for each deferred value stream, look up the sibling in-map bitmap and popcount `[0, rowRange.startRow)` -> encoded offset, `[rowRange.startRow, rowRange.endRow)` -> encoded length, then slice. In-map omitted -> all-present -> encoded coordinates match top-level. Empty encoded length -> skip (value stream contributes nothing). Sliced bytes live in a per-Deserializer `Buffer` (`sliceBuffer_`) whose lifetime mirrors `parser_`'s stripped-buffer references: both reset at the end of each `deserialize*` call and at each null-barrier boundary. ## Downstream migration bundled in this diff Zero non-test/non-benchmark production callers rely on the old over-fetch semantics. Test/benchmark callers that construct kTablet batches and consume Deserializer output are all updated here: - `dwio/nimble/velox/index/tests/NimbleIndexProjectorTest.cpp` — 13 tests (52 param combinations) updated from over-fetch expectations to sliced-output expectations; `expectVectorRows` helper simplified. - `rocks/nimble/tests/NimbleTableTest.cpp` — 8 sites updated: helper docstring, ASSERT_EQ against `rowRange.numRows()`, loops iterate `[0, rowRange.numRows())` instead of `[startRow, endRow)`. - `zippydb/server/tests/ZippyDBNimbleExecuteIntegrationTest.cpp` — 1 site in `decodeLogicalRowsFromChunk`. - `rexdb/benchmarks/file_format/lib/ZippyDbBenchmarkReader.cpp` — 1 benchmark iteration loop. - `rexdb/benchmarks/file_format/lib/NimbleIndexBenchmarkReader.cpp` — `coalesceChunkSlice` docstring. - `rocks/nimble/docs/nimble-external-table-design.md` — over-fetch note rewritten to describe the new contract. Callers under `rocks/nimble/tests/NimbleResultIOBufTest.cpp`, `zippydb/rodos/storage/tests/NimbleRodosDBTest.cpp`, and the aggregation loop in `ZippyDBNimbleExecuteIntegrationTest.cpp` lines 2108-2138 only aggregate `rowRange.numRows()` — no code changes needed (the new contract is transparent to them). Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 2, 2026 07:33
32227aa to
587940f
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Aug 2, 2026
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 2, 2026 07:42
587940f to
fe40146
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Aug 2, 2026
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 2, 2026 07:43
fe40146 to
8d04574
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Aug 3, 2026
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 3, 2026 03:44
8d04574 to
a5c1a05
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Aug 3, 2026
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 3, 2026 04:34
a5c1a05 to
5973105
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Aug 3, 2026
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 3, 2026 04:39
5973105 to
6222212
Compare
…kincubator#1045) Summary: `nimble::Deserializer` now honors the per-batch `rowRange` embedded in each kTablet chunk header — the decoded output contains only the rows the projector requested. Uses `SegmentedStreamDecoder::skip` + `reader_->next` to bypass rows outside the range without materializing them. Adds a public overload: ``` void deserialize( const std::vector<std::string_view>& data, const std::vector<nimble::RowRange>& rowRanges, velox::VectorPtr& output) const; ``` Caller-supplied ranges override any rowRange encoded in the header. Also bundles the downstream test/benchmark/doc updates that previously relied on the over-fetch behavior — `dwio/nimble/velox/index/tests`, `rocks/nimble/tests`, `zippydb/server/tests`, `rexdb/benchmarks/file_format/lib`, and `rocks/nimble/docs`. Companion diff D114257287 implements the same behavior via a slice-at- ingest approach; kept side by side for comparison. Reviewed By: xiaoxmeng Differential Revision: D113966951
tanjialiang
force-pushed
the
export-D113966951
branch
from
August 3, 2026 06:20
6222212 to
96c3835
Compare
|
This pull request has been merged in 8655f92. |
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
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:
nimble::Deserializernow honors the per-batchrowRangeembedded in eachkTablet chunk header — the decoded output contains only the rows the
projector requested. Uses
SegmentedStreamDecoder::skip+reader_->nextto bypass rows outside the range without materializing them.
Adds a public overload:
Caller-supplied ranges override any rowRange encoded in the header.
Also bundles the downstream test/benchmark/doc updates that previously
relied on the over-fetch behavior —
dwio/nimble/velox/index/tests,rocks/nimble/tests,zippydb/server/tests,rexdb/benchmarks/file_format/lib, androcks/nimble/docs.Companion diff D114257287 implements the same behavior via a slice-at-
ingest approach; kept side by side for comparison.
Reviewed By: xiaoxmeng
Differential Revision: D113966951