Repository navigation
perf(query): selectively index unique null-fill fields - #54
Merged
Merged
Conversation
VibeSWE
marked this pull request as ready for review
October 21, 2026 13:41
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.
For
fill_null=truewith nonempty, unique requested fields, append missing nulls by direct requested-field index. Reuse the existing map and preserve the original writer body for duplicate or empty requests andfill_null=false. This avoids repeated map lookups in the selected null-fill loop. Add six regression tests covering byte order, missing/duplicate fields, nullable rows, changing schemas, empty/columnless batches and fallback behavior.Validation uses retained evidence for the exact production and test sources: 22 baseline + 22 candidate tests passed in the component harness; nine fixtures showed exact JSON and heap parity over 20 calls per arm. The component harness uses a DataFusion shim and a separate compilation profile. The ordinary-release combined application completed the full campaign: 18 lifetimes, 90 cells and 84,600 timed responses; all native exits were zero and closure was independently verified.
In that campaign, C/B isolates this serializer on the same correctness-fixed source and matched plans/bytes/queries. Across three blocks, process CPU/query fell 4.9–5.2% for H-fill and 10.6–11.3% for W-unique. Default-path performance remains unresolved (C/B 0.997–1.011); application memory remains unresolved. These are descriptive results from one host with fixed datasets and concurrency 1, not a blanket no-regression or adoption claim. This PR contains only the serializer and tests; it is separately reviewable from the sorting correctness fix.
Full-crate validation now passes on
026c0615d896b297ee9e9cda80acecfae3d25daa: Rust tests, Clippy, image build, and standalone/distributed smoke tests are green (CI run). The standalone branch has not been separately performance-measured; the measured application was the combined build including the reader fix in #48. The retained component measurements and the full-crate CI checks are distinct evidence.The helper deliberately copies the original writer body to keep the fallback source unchanged; future writer fixes must keep the two paths consistent. The user authorized merge on 2026-10-21 after reviewing the disclosed tradeoff. PR #48 is merged. CI passed again on the combined head
6acbbffe2f6a04e2fb2a02344902f5493bb7d7eebefore this PR was merged (combined CI run). Successful CI does not establish application memory parity.Need help on this PR? Tag
@codesmith-botwith what you need.