From 7836521d7ecbf823e0062bb6cd6417f465796564 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Sun, 13 Sep 2026 08:00:46 +0800 Subject: [PATCH 1/4] [common] Widen hilbert index bytes beyond 8 dimensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../paimon/sort/hilbert/HilbertIndexer.java | 10 +++++- .../sort/hilbert/HilbertIndexerTest.java | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java b/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java index 21f725015edb..c1017a45d9dc 100644 --- a/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java +++ b/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java @@ -317,7 +317,15 @@ public static byte[] hilbertCurvePosBytes(Long[] points) { long[] data = Arrays.stream(points).mapToLong(Long::longValue).toArray(); HilbertCurve hilbertCurve = HilbertCurve.bits(BITS_NUM).dimensions(points.length); BigInteger index = hilbertCurve.index(data); - return ConvertBinaryUtil.paddingToNByte(index.toByteArray(), BITS_NUM); + // an N-dimensional 63-bit index needs up to 63*N/8 + 1 bytes (the extra one is + // BigInteger's sign byte when the top bit is set); keep the legacy 63-byte width + // for up to 8 dimensions — which still truncates the low byte for the half of the + // space with the top bit set, preserved only for byte stability of existing keys — + // and use the full width beyond that instead of silently dropping low-order bits. + // Hilbert keys are transient in all current consumers, so nothing persists the + // legacy shape. + int paddingBytes = data.length <= 8 ? BITS_NUM : BITS_NUM * data.length / 8 + 1; + return ConvertBinaryUtil.paddingToNByte(index.toByteArray(), paddingBytes); } /** Process function interface. */ diff --git a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java index 238e80cbb5f6..62e0ba7f4c8d 100644 --- a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java @@ -60,6 +60,40 @@ public void testBooleanValuesDistinctFromNull() { assertThat(falseIndex).isNotEqualTo(trueIndex); } + @Test + public void testHighDimensionIndexKeepsAllBits() { + // 9 dimensions: the 63*9-bit index needs 71 bytes; distinct points that differ + // only in the low-order bits must stay distinct instead of being truncated away + Long[][] points = { + {0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L}, + {0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 1L}, + }; + byte[] first = HilbertIndexer.hilbertCurvePosBytes(points[0]); + byte[] second = HilbertIndexer.hilbertCurvePosBytes(points[1]); + assertThat(first).hasSize(71); + assertThat(second).hasSize(71); + assertThat(first).isNotEqualTo(second); + + // 16 dimensions: the top bit being set adds BigInteger's sign byte, so the width + // must cover it or the low byte is truncated away + Long[] highBits = + new Long[] { + Long.MAX_VALUE, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L + }; + Long[] highBitsVariant = highBits.clone(); + highBitsVariant[15] = 1L; + byte[] highFirst = HilbertIndexer.hilbertCurvePosBytes(highBits); + byte[] highSecond = HilbertIndexer.hilbertCurvePosBytes(highBitsVariant); + assertThat(highFirst).hasSize(127); + assertThat(highSecond).hasSize(127); + assertThat(highFirst).isNotEqualTo(highSecond); + + // up to 8 dimensions keep the legacy 63-byte width, so existing indexes are stable + assertThat(HilbertIndexer.hilbertCurvePosBytes(new Long[] {0L, 0L})).hasSize(63); + assertThat(HilbertIndexer.hilbertCurvePosBytes(new Long[] {0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L})) + .hasSize(63); + } + private static GenericRow booleanRow(Boolean value) { GenericRow row = new GenericRow(2); row.setField(0, value); From b90005113fad553cb5c495fe8c034d7c0439b696 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Sun, 13 Sep 2026 12:44:42 +0800 Subject: [PATCH 2/4] fix: widen hilbert keys at every dimension count 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 --- .../paimon/sort/hilbert/HilbertIndexer.java | 15 +++-- .../sort/hilbert/HilbertIndexerTest.java | 58 ++++++++++++++++++- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java b/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java index c1017a45d9dc..b4d67e48cabc 100644 --- a/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java +++ b/paimon-common/src/main/java/org/apache/paimon/sort/hilbert/HilbertIndexer.java @@ -317,14 +317,13 @@ public static byte[] hilbertCurvePosBytes(Long[] points) { long[] data = Arrays.stream(points).mapToLong(Long::longValue).toArray(); HilbertCurve hilbertCurve = HilbertCurve.bits(BITS_NUM).dimensions(points.length); BigInteger index = hilbertCurve.index(data); - // an N-dimensional 63-bit index needs up to 63*N/8 + 1 bytes (the extra one is - // BigInteger's sign byte when the top bit is set); keep the legacy 63-byte width - // for up to 8 dimensions — which still truncates the low byte for the half of the - // space with the top bit set, preserved only for byte stability of existing keys — - // and use the full width beyond that instead of silently dropping low-order bits. - // Hilbert keys are transient in all current consumers, so nothing persists the - // legacy shape. - int paddingBytes = data.length <= 8 ? BITS_NUM : BITS_NUM * data.length / 8 + 1; + // an N-dimensional 63-bit index needs up to 63*N/8 + 1 bytes: the extra byte covers + // BigInteger's sign byte when the top bit is set. paddingToNByte drops trailing bytes + // when the array is longer than the width, so a narrower width silently discards + // low-order bits — at 8 dimensions that also inverts the order, because the sign byte + // makes an upper-half index sort below a smaller lower-half one. Hilbert keys are + // transient sort keys in every consumer, so widening them breaks nothing. + int paddingBytes = BITS_NUM * data.length / 8 + 1; return ConvertBinaryUtil.paddingToNByte(index.toByteArray(), paddingBytes); } diff --git a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java index 62e0ba7f4c8d..3bbd03bf931a 100644 --- a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java @@ -23,9 +23,13 @@ import org.apache.paimon.types.DataTypes; import org.apache.paimon.types.RowType; +import org.davidmoten.hilbert.HilbertCurve; import org.junit.jupiter.api.Test; +import java.math.BigInteger; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -88,10 +92,58 @@ public void testHighDimensionIndexKeepsAllBits() { assertThat(highSecond).hasSize(127); assertThat(highFirst).isNotEqualTo(highSecond); - // up to 8 dimensions keep the legacy 63-byte width, so existing indexes are stable - assertThat(HilbertIndexer.hilbertCurvePosBytes(new Long[] {0L, 0L})).hasSize(63); + // the width is 63*N/8 + 1 for every N, so a 2-dimension key is 16 bytes and an + // 8-dimension one is 64 — the extra byte over the 63-byte magnitude is what makes + // room for BigInteger's sign byte + assertThat(HilbertIndexer.hilbertCurvePosBytes(new Long[] {0L, 0L})).hasSize(16); assertThat(HilbertIndexer.hilbertCurvePosBytes(new Long[] {0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L})) - .hasSize(63); + .hasSize(64); + } + + /** + * At 8 dimensions the index fills 63 bytes, so the top half of the space carries + * BigInteger's sign byte and spills to 64. Truncating that back to 63 does not merely lose + * resolution: the leading zero makes a large index sort below a smaller one. + */ + @Test + public void testEightDimensionKeysOrderLikeTheirIndex() { + List points = new ArrayList<>(); + for (long i = 0; i < 24; i++) { + // spread the points over the whole space so some land in the top half + long v = Long.MAX_VALUE / 23 * i; + points.add(new Long[] {v, v / 3, i, Long.MAX_VALUE - v, v / 7, i * 31, v / 11, i}); + } + + for (Long[] left : points) { + for (Long[] right : points) { + int indexOrder = index(left).compareTo(index(right)); + int keyOrder = + compareUnsigned( + HilbertIndexer.hilbertCurvePosBytes(left), + HilbertIndexer.hilbertCurvePosBytes(right)); + assertThat(Integer.signum(keyOrder)) + .as( + "key order must follow index order for %s vs %s", + Arrays.toString(left), Arrays.toString(right)) + .isEqualTo(Integer.signum(indexOrder)); + } + } + } + + private static BigInteger index(Long[] points) { + long[] data = Arrays.stream(points).mapToLong(Long::longValue).toArray(); + return HilbertCurve.bits(63).dimensions(points.length).index(data); + } + + private static int compareUnsigned(byte[] left, byte[] right) { + assertThat(left).hasSameSizeAs(right); + for (int i = 0; i < left.length; i++) { + int cmp = Integer.compare(left[i] & 0xFF, right[i] & 0xFF); + if (cmp != 0) { + return cmp; + } + } + return 0; } private static GenericRow booleanRow(Boolean value) { From 2e10334fdd0148b417d5d5da995834c518ab653c Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Sun, 13 Sep 2026 12:54:16 +0800 Subject: [PATCH 3/4] style: spotless javadoc reflow Co-Authored-By: Claude Code --- .../org/apache/paimon/sort/hilbert/HilbertIndexerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java index 3bbd03bf931a..fcd4b23f996f 100644 --- a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java @@ -101,9 +101,9 @@ public void testHighDimensionIndexKeepsAllBits() { } /** - * At 8 dimensions the index fills 63 bytes, so the top half of the space carries - * BigInteger's sign byte and spills to 64. Truncating that back to 63 does not merely lose - * resolution: the leading zero makes a large index sort below a smaller one. + * At 8 dimensions the index fills 63 bytes, so the top half of the space carries BigInteger's + * sign byte and spills to 64. Truncating that back to 63 does not merely lose resolution: the + * leading zero makes a large index sort below a smaller one. */ @Test public void testEightDimensionKeysOrderLikeTheirIndex() { From 5e8bf23e0f6d6576cb236156387ee352585d4bf7 Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Sun, 13 Sep 2026 15:08:36 +0800 Subject: [PATCH 4/4] test: assert the hilbert key round-trips its index 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 --- .../sort/hilbert/HilbertIndexerTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java index fcd4b23f996f..f547edb2e18a 100644 --- a/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/sort/hilbert/HilbertIndexerTest.java @@ -130,6 +130,28 @@ public void testEightDimensionKeysOrderLikeTheirIndex() { } } + /** + * Sizes and inequalities are proxies for the property the width exists to guarantee: the key + * carries the whole index. State it directly, at the two dimension counts where the index needs + * its last byte the most. + */ + @Test + public void testKeyCarriesTheWholeIndex() { + for (int dimensions : new int[] {8, 9}) { + Long[] topOfSpace = new Long[dimensions]; + Arrays.fill(topOfSpace, Long.MAX_VALUE); + Long[] oneLowBitOff = topOfSpace.clone(); + oneLowBitOff[dimensions - 1] = Long.MAX_VALUE - 1; + + for (Long[] point : new Long[][] {topOfSpace, oneLowBitOff}) { + byte[] key = HilbertIndexer.hilbertCurvePosBytes(point); + assertThat(new BigInteger(1, key)) + .as("key must round-trip the index at %s dimensions", dimensions) + .isEqualTo(index(point)); + } + } + } + private static BigInteger index(Long[] points) { long[] data = Arrays.stream(points).mapToLong(Long::longValue).toArray(); return HilbertCurve.bits(63).dimensions(points.length).index(data);