Fix node_16::grow() missing 128 offset in node_48 index population#29
Merged
rafaelkallis merged 3 commits intomainfrom Nov 24, 2025
Merged
Fix node_16::grow() missing 128 offset in node_48 index population#29rafaelkallis merged 3 commits intomainfrom
rafaelkallis merged 3 commits intomainfrom
Conversation
node_16::grow() populates node_48::indexes_[] without the required 128 offset, causing child lookups to fail after growth. All other node_48 operations use indexes_[128 + partial_key] for consistent byte-to-index mapping. Changes: - include/art/node_16.hpp: Add 128 offset when populating node_48::indexes_[] during growth - test/node_16.cpp: Add test case that grows node_16 to node_48 and verifies child accessibility and iterator navigation - .gitignore: Exclude _codeql_detected_source_root Impact: Without this fix: - Children inserted before growth become unreachable - Iterator navigation returns incorrect keys - Data effectively corrupted after any node_16→node_48 transition Related to PR #20. Co-authored-by: bowling233 <zhubaolin228@gmail.com>
Copilot
AI
changed the title
[WIP] Recreate PR 26 and add co-author
Fix node_16::grow() missing 128 offset in node_48 index population
Nov 23, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in node_16::grow() that caused data corruption when a node_16 transitions to node_48. The fix ensures children remain accessible after node growth by correctly applying the 128 offset required by node_48's indexes_[] array, which maps signed char partial keys (-128 to 127) to array indices (0 to 255).
Key Changes:
- Fixed
node_16::grow()to useindexes_[128 + partial_key]instead ofindexes_[partial_key], making it consistent with all other node_48 operations - Added comprehensive test case that verifies children remain accessible after growth and that iteration (next_partial_key/prev_partial_key) works correctly
- Updated
.gitignoreto exclude CodeQL temporary file
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
include/art/node_16.hpp |
Adds 128 offset when populating node_48's indexes_ array during node growth, fixing the data corruption bug |
test/node_16.cpp |
Adds test case that verifies children accessibility and iteration after node_16→node_48 growth, but has a SUBCASE placement bug causing memory safety issues |
.gitignore |
Excludes CodeQL-generated temporary file _codeql_detected_source_root |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Recreates PR #26 with co-author attribution for bowling233.
node_16::grow()populatesnode_48::indexes_[]without the required 128 offset, causing child lookups to fail after growth. All othernode_48operations consistently useindexes_[128 + partial_key]for byte-to-index mapping.Changes
include/art/node_16.hpp: Add 128 offset when populatingnode_48::indexes_[]during growthtest/node_16.cpp: Add test case that grows node_16 to node_48 and verifies child accessibility and iterator navigation.gitignore: Exclude_codeql_detected_source_rootImpact
Without this fix, children inserted before growth become unreachable, iterator navigation returns incorrect keys, and data is effectively corrupted after any node_16→node_48 transition.
Related to PR #20.
Co-authored-by: bowling233 zhubaolin228@gmail.com
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.