fix(index): preserve range partition structure in BTree index update during optimize#6904
Draft
Jay-ju wants to merge 4 commits into
Draft
fix(index): preserve range partition structure in BTree index update during optimize#6904Jay-ju wants to merge 4 commits into
Jay-ju wants to merge 4 commits into
Conversation
…during optimize When a BTree index is built with range partitioning (distributed mode), the update() method previously called combine_old_new() which is unaware of the range partition structure. This caused the retrained index to lose its range partition layout, producing a single monolithic index instead of per-partition page data and lookup files. This fix detects range-partitioned indices by checking ranges_to_files and, when present, re-trains each partition independently before merging the per-partition lookup files into a unified lookup with correct page_idx offsets. Added tests: - test_optimize_btree_after_append_preserves_data: verifies regular BTree index data correctness after append + optimize - test_optimize_btree_index_update_preserves_range_partition_structure: verifies range partition structure is preserved after optimize
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…e-partitioned BTree optimize - Enhanced test_optimize_btree_index_update_preserves_range_partition_structure to verify range query and point query correctness after optimize - Added test_optimize_btree_range_partition_with_three_partitions to cover 3-partition scenario with data that doesn't divide evenly, verifying both structure preservation and query correctness across partition boundaries
…structure preservation The existing test asserted that update() on a range-partitioned BTree index should fall back to non-ranged. After the fix that preserves range partition structure during update, this assertion must be updated to expect ranges_to_files.is_some() instead.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
3 tasks
Contributor
Author
|
@claude review |
…atches in range-partitioned BTree update - Track trained_partition_ids instead of assuming all partitions are trained - Skip merge when only one partition was trained - Remove collect_sorted_batches helper, inline the collection logic - Pass SchemaRef to batches_to_stream to avoid accessing empty batch vector
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.
Problem
When a BTree index is built with range partitioning (distributed mode), the
update()method previously calledcombine_old_new()which is unaware of the range partition structure. This caused the retrained index to lose its range partition layout during optimize, producing a single monolithic index instead of per-partition page data and lookup files.Root Cause
BTreeIndex::update()unconditionally callscombine_old_new()+train_btree_index()without checking whether the index was built with range partitioning. Thetrain_btree_index()function withrange_id=Nonegenerates a non-partitioned index, destroying the original range partition structure.Fix
Detect range-partitioned indices by checking
ranges_to_filesinupdate(). When present:combine_old_new()range_idpage_idxoffsets viamerge_range_partition_lookups_in_place()New Helper Functions
collect_sorted_batches()— collects aSendableRecordBatchStreamintoVec<RecordBatch>slice_batches()— slices a batch vector by row range[start, end)batches_to_stream()— convertsVec<RecordBatch>back toSendableRecordBatchStreammerge_range_partition_lookups_in_place()— merges per-partition lookup files into a unified lookup, adjustingpage_idxoffsets and writingpages_per_range_partitionmetadataTests
test_optimize_btree_after_append_preserves_data— verifies regular BTree index data correctness after append + optimizetest_optimize_btree_index_update_preserves_range_partition_structure— verifies range partition structure (part_*files) is preserved after optimize