Skip to content

[common] Order decimal z-values through the signed long transform - #9626

Open
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/zindexer-decimal-order
Open

[common] Order decimal z-values through the signed long transform#9626
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/zindexer-decimal-order

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Purpose

close #9625

ZIndexer's DECIMAL branch fed Decimal.toUnscaledBytes(), a minimal two's-complement array, into the fixed 8-byte z-value buffer:

ZOrderByteUtils.byteTruncateOrFill(((Decimal) o).toUnscaledBytes(), PRIMITIVE_BUFFER_SIZE, reuse)

byteTruncateOrFill left-aligns and zero-pads, and z-values are compared unsigned, so the encoding is not order-preserving. On DECIMAL(20,2), -1.00 becomes 9C 00 ... (156 unsigned) and 1.00 becomes 64 00 ... (100), putting the negative above the positive; 100.00 becomes 27 10 00 ... and lands below 1.00 because it is longer; and 0.00 becomes eight zero bytes, which is exactly the NULL_BYTES sentinel.

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_VALUE itself is left out of the range because longToOrderedBytes maps 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.testZIndexerDecimalOrdering builds a two-column DECIMAL(20,2) ZIndexer and 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.00 encodes above 0.00.

mvn -pl paimon-common -Dtest=TestZOrderByteUtil test on JDK 8: 15 tests, 0 failures. spotless:check and checkstyle:check on paimon-common are clean.

Not included: HilbertIndexer encodes decimals as toBigDecimal().longValue(), dropping the fraction, so every magnitude below 1 collapses to one key. Same area, different bug, and it deserves its own change.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Decimal z-order values are not order-preserving and zero collides with the null sentinel

1 participant