JingsongLi commented on code in PR #9626:
URL: https://github.com/apache/paimon/pull/9626#discussion_r3986509559
##########
paimon-common/src/main/java/org/apache/paimon/sort/zorder/ZIndexer.java:
##########
@@ -239,15 +240,30 @@ public ZProcessFunction visit(VarBinaryType
varBinaryType) {
public ZProcessFunction visit(DecimalType decimalType) {
final InternalRow.FieldGetter fieldGetter =
InternalRow.createFieldGetter(decimalType, fieldIndex);
+ // An unscaled value of a DECIMAL(p, s) is up to p digits wide,
past what a long holds
+ // for p > 18. Right-shift it into the signed-long range by a
per-column amount before
+ // the long transform: an arithmetic shift by a fixed width is a
monotonic projection
+ // that keeps the high-order bits, so separated values keep
distinct z-keys instead of
+ // saturating to one bound the way clamping did. The widest
magnitude for the column is
+ // below 10^p, so shifting off (bitLength(10^p) - 63) bits leaves
at most 63, which fits
+ // a long without reaching Long.MIN_VALUE (whose z-value is the
null sentinel). Compact
+ // decimals (p <= 18) always fit, so the shift is 0.
+ final int unscaledShift =
+ Math.max(0,
BigInteger.TEN.pow(decimalType.getPrecision()).bitLength() - 63);
Review Comment:
[P2] Preserve decimal clustering resolution in ordinary value ranges
For DECIMAL(38,18), this shift is 64, so every nonnegative value below
18.446744073709551616 maps to long 0: 0/1/2/3/10 all have the same Z-key when
the other column is fixed. Decimal.isCompact() depends on declared precision,
so none escape via the compact branch. An exact base/head probe reduced 10,001
values in [0,10] from 9,962 keys to one, and the actual core ZorderSorter
changed a repeated 1/2/3/10 input from four groups into mixed output because it
compares only this key. Price/rate columns using a wide decimal schema
therefore lose useful clustering while still paying the rewrite cost. Please
use a monotonic representation with useful relative resolution (or a wider
sortable decimal encoding), and test small separated values and negative
counterparts as well as values beyond long bounds. The 10/20/90 test crosses
the new coarse buckets and misses this regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
