[common] Order decimal z-values through the signed long transform - #9626
Open
LuciferYang wants to merge 1 commit into
Open
[common] Order decimal z-values through the signed long transform#9626LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
The DECIMAL branch wrote Decimal.toUnscaledBytes(), a variable-length two's-complement array, into the fixed 8-byte z-value buffer, which is compared unsigned. On DECIMAL(20,2) that put -1.00 above 1.00, put 100.00 below 1.00 because it is longer, and made 0.00 encode to the all-zero null sentinel. Encode the unscaled value with longToOrderedBytes, like the other numeric types. Unscaled values wider than a long are clamped rather than narrowed, so the mapping stays non-decreasing; the lower bound stops one above Long.MIN_VALUE, which would encode to the sentinel.
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
close #9625
ZIndexer's DECIMAL branch fedDecimal.toUnscaledBytes(), a minimal two's-complement array, into the fixed 8-byte z-value buffer:byteTruncateOrFillleft-aligns and zero-pads, and z-values are compared unsigned, so the encoding is not order-preserving. On DECIMAL(20,2),-1.00becomes9C 00 ...(156 unsigned) and1.00becomes64 00 ...(100), putting the negative above the positive;100.00becomes27 10 00 ...and lands below1.00because it is longer; and0.00becomes eight zero bytes, which is exactly theNULL_BYTESsentinel.Decimals now go through the same sign-flipped fixed-width transform as the other numeric types,
ZOrderByteUtils.longToOrderedBytes, on the unscaled value. Scale is fixed per column, so ordering by unscaled value is ordering by numeric value.Unscaled values wider than a long are clamped into
[Long.MIN_VALUE + 1, Long.MAX_VALUE]instead of being narrowed, which keeps the mapping non-decreasing: the extremes collapse onto each other rather than wrapping around and sorting below small values.Long.MIN_VALUEitself is left out of the range becauselongToOrderedBytesmaps it to eight zero bytes, which is the null sentinel.This only changes the sort key used while clustering, so nothing on disk changes format and no data is rewritten differently beyond the intended layout.
Tests
TestZOrderByteUtil.testZIndexerDecimalOrderingbuilds a two-column DECIMAL(20,2)ZIndexerand compares whole interleaved z-values unsigned:(-1.00, 0.00)must sort below(0.00, 1.00), and the all-null row must sort below both, which pins zero apart from the sentinel. Two more rows cover the clamp: an unscaled value too wide for a long sorts above the small positives, its negative counterpart sorts below the small negatives, and both stay above the null sentinel.Against the unfixed indexer the first comparison already fails, since
-1.00encodes above0.00.mvn -pl paimon-common -Dtest=TestZOrderByteUtil teston JDK 8: 15 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.Not included:
HilbertIndexerencodes decimals astoBigDecimal().longValue(), dropping the fraction, so every magnitude below 1 collapses to one key. Same area, different bug, and it deserves its own change.