[common] Widen hilbert index bytes beyond 8 dimensions - #9777
Draft
LuciferYang wants to merge 6 commits into
Draft
[common] Widen hilbert index bytes beyond 8 dimensions#9777LuciferYang wants to merge 6 commits into
LuciferYang wants to merge 6 commits into
Conversation
hilbertCurvePosBytes padded the N-dimensional 63-bit hilbert index to a fixed 63 bytes. That is only enough for up to 8 dimensions — and even there it truncates the low byte for the half of the space whose index has the top bit set (BigInteger's sign byte), e.g. any row with a NULL order column. With 9+ order columns — trivially configurable via the hilbert sorter and the Spark Hilbert UDF — the padding dropped entire low-order bytes, collapsing distinct points into the same sort key. Keep the legacy 63-byte width for up to 8 dimensions so existing index bytes stay stable, and use the full 63*N/8 + 1 width beyond that. Hilbert keys are transient in all current consumers, so nothing persists the legacy shape. Assisted-by: GLM-5.3
LuciferYang
marked this pull request as draft
September 13, 2026 03:07
Keeping the legacy 63-byte width for up to 8 dimensions preserved a real defect rather than compatibility: at exactly 8 dimensions the index fills 63 bytes, so the top half of the space carries BigInteger's sign byte and spills to 64, and truncating back to 63 leaves a leading zero that makes a large index sort BELOW a smaller one. The keys are transient sort keys in every consumer, so there is nothing to stay byte-compatible with; use 63*N/8 + 1 everywhere, which also shrinks a 2-dimension key from 63 bytes to 16. Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Sizes and inequalities are proxies for the property the width exists to guarantee. Assert new BigInteger(1, key) equals the index at 8 and 9 dimensions, where the last byte is exactly what the old width dropped. Co-Authored-By: Claude Code <noreply@anthropic.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
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.
Purpose
close #9776
HilbertIndexer.hilbertCurvePosBytespadded the index to a fixed 63 bytes.ConvertBinaryUtil.paddingToNBytekeeps the first n bytes when the array is longer, so it drops low-order bytes off a big-endian value. An N-dimensional 63-bit index needs up to63*N/8 + 1bytes, the extra byte covering BigInteger's sign byte when the top bit is set, so 63 was too narrow from 8 dimensions upward and the sort key silently lost resolution.At 8 dimensions this is worse than lost resolution. The index fills exactly 63 bytes, so a value in the top half of the space serializes to 64 bytes with a leading
0x00. Truncating that back to 63 leaves the zero in front, and the key then sorts below the untruncated key of a numerically smaller index. Clustering does not just get coarser, it gets the order backwards for half the space.The width is now
63*N/8 + 1at every dimension count. Nothing needs the old shape: all three consumers (HibertSorterin paimon-core,HilbertSorterin paimon-flink,SparkHilbertUDF) use the key as a transient sort key and strip it before writing, so no data file, manifest or index file carries it, and two keys are only ever compared within one sort where the dimension count is fixed. Ordering is unaffected for 1 to 7 dimensions, where 63 bytes never truncated: those keys just stop carrying dozens of leading zero bytes, so a 2-dimension key goes from 63 bytes to 16.Tests
HilbertIndexerTest.testHighDimensionIndexKeepsAllBitsasserts the widths (16 for 2 dimensions, 64 for 8, 71 for 9, 127 for 16) and that points differing only in low-order bits stay distinct at 9 and 16 dimensions.testEightDimensionKeysOrderLikeTheirIndexis the one that pins the ordering bug: it spreads 24 points across the space, recomputes each index withHilbertCurvedirectly, and asserts that comparing the encoded keys unsigned agrees with comparing the indexes for every pair.Verified on JDK 11: all 3 tests pass with the change; restoring the 63-byte width for 8 dimensions and nothing else makes
testEightDimensionKeysOrderLikeTheirIndexfail on a concrete pair, alongside the width assertion.paimon-commonbuilds clean with checkstyle and spotless.