feat(shredding): add shared-shredding map placement policies#384
Open
zjw1111 wants to merge 2 commits into
Open
feat(shredding): add shared-shredding map placement policies#384zjw1111 wants to merge 2 commits into
zjw1111 wants to merge 2 commits into
Conversation
zjw1111
force-pushed
the
codex/extend-map-column-allocator
branch
4 times, most recently
from
3d6ce68 to
534ca95
Compare
zjw1111
force-pushed
the
codex/extend-map-column-allocator
branch
from
534ca95 to
fc44b2d
Compare
lxy-9602
reviewed
Jun 27, 2026
| (last_used_[col] == selected_last_used && col < selected_col))) { | ||
| selected_col = col; | ||
| selected_last_used = last_used_[col]; | ||
| } |
Collaborator
There was a problem hiding this comment.
Please add comments for the strategy.
| if (is_empty) { | ||
| if (!selected_empty || col < selected_col) { | ||
| selected_empty = true; | ||
| selected_col = col; |
Collaborator
There was a problem hiding this comment.
I’m not quite following the purpose of the defensive check col < selected_col here. At the current call site, candidates is constructed in ascending column order, so could we just return as soon as we find a suitable empty one?
| ])") | ||
| .ValueOrDie(); | ||
|
|
||
| AssertArrayEquals(expected, actual); |
Collaborator
There was a problem hiding this comment.
Please also check BuildFieldMeta.
| PLAIN = 0, | ||
| /// Sort field IDs and place fields into columns 0..K-1. | ||
| SEQUENTIAL = 1, | ||
| /// Choose empty columns first, then the least-recently-used physical column. |
Collaborator
There was a problem hiding this comment.
I’m not sure this wording is the best fit here. These field IDs are not Paimon field IDs, but an internal shared-shedding concept that is not exposed to users. Could we rewrite the comment to better reflect the user-facing behavior, or add a simple example instead?
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.
Purpose
Linked issue: #342
Add shared-shredding map column placement policies for the write path:
fields.<column>.map.shared-shredding.column-placement-policy, defaulting tolru.MapSharedShreddingContextand letMapSharedShreddingBatchConverterread placement policy fromCoreOptionswhen creating per-column allocators.Tests
cmake --build build --target paimon-common-test -j64cmake --build build --target paimon-core-test -j64env LSAN_OPTIONS=detect_leaks=0 build/debug/paimon-common-test --gtest_filter=*MapSharedShredding*env LSAN_OPTIONS=detect_leaks=0 build/debug/paimon-core-test --gtest_filter=CoreOptionsTest.TestDefaultValue:CoreOptionsTest.TestMapStorageLayout:*SharedShredding*git diff --checkgit diff --cached --checkAPI and Format
Adds a core option for selecting the shared-shredding map column placement policy. The physical shared-shredding map column assignment can vary by configured policy; the default policy is
lru.Documentation
No.
Generative AI tooling
Generated-by: OpenAI Codex (GPT-5)