-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NodeGroup list storage refactor #1885
Merged
Merged
Conversation
This file contains 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
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1885 +/- ##
==========================================
- Coverage 89.50% 89.26% -0.24%
==========================================
Files 834 831 -3
Lines 30482 30491 +9
==========================================
- Hits 27283 27219 -64
- Misses 3199 3272 +73
☔ View full report in Codecov by Sentry. |
acquamarin
force-pushed
the
node-group-list
branch
2 times, most recently
from
August 3, 2023 01:51
c44cd94
to
072b545
Compare
ray6080
approved these changes
Aug 3, 2023
acquamarin
force-pushed
the
node-group-list
branch
from
August 3, 2023 19:42
072b545
to
2925928
Compare
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.
This PR refactors the list storage structure using a nested dataColumn design:
Each list contains two columns: offsetColumn, dataColumn.
OffsetColumn: stores the endOffset in the dataColumn for each list element.
DataColumn: stores the actual elements of the list, indexed by the offsetColumn.
For example: store [1,3,null,4],null,[5,7],[]:
OffsetColumn: [4, 4 6, 6]
NullColumn: [false, true, false, false]
DataColumn: [1,3, null, 4, 5, 7]
To read lists:
Note: we always read the current offset and the prev offset for the element. The current offset indicates the endOffset(exclusive) in the dataVector for the current element and the prev offset indicates the startOffset(inclusive) in the dataVector for the current element.
For example: we want to read the 3rd element, we read two offsets:
offset at 2nd pos: offsetColumn[2]: 4
offset at 3rd pos: offsetColumn[3]: 6
Then we can figure out that, the 3rd element's data starts at 4 and ends at 6 in the data column.
1st lst: offset 4 means the first list data ends at offset 4(exclusive) in the dataColumn. So the 1st list data is stored in position 0~4(which is [1,3,null,4]).
2nd lst: offset 4 menas the second list ends at offset 4(exclusive) in the dataColumn, however the nullColumn suggests that it is null. So the 2nd lst is null.
3rd lst: offset 6 means the third list data ends at offset 6(exclusive) and 4 means the third list data starts at offset4(inclusive). So the 3rd list data is [5,7].
4th lst: offset 6 means the fourth list dataends at offset 6(exclusive) and 6 means the third list data starts at offset6(inclusive). So the 4th list is an empty list.