From c04ae035008e37c28cd31c4c2130092a555b62f3 Mon Sep 17 00:00:00 2001 From: Jordan Epstein Date: Mon, 27 Jul 2026 16:02:34 -0500 Subject: [PATCH] [common] Fix SliceComparator on BINARY and VARBINARY fields (#8873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TypeCheckUtils.isComparable admits BINARY and VARBINARY, and primary keys may contain them, but SliceComparator cast every field value to Comparable — byte[] is not, so any sorted-lookup-store comparison over a binary key field (lookup compaction, deletion-vector maintenance, lookup changelog producer) threw ClassCastException. Compare binary fields with SortUtil.compareBinary, the same unsigned lexicographic order BinaryRow uses. --- .../serializer/RowCompactedSerializer.java | 13 +++++- .../RowCompactedSerializerTest.java | 42 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/paimon-common/src/main/java/org/apache/paimon/data/serializer/RowCompactedSerializer.java b/paimon-common/src/main/java/org/apache/paimon/data/serializer/RowCompactedSerializer.java index 4ceffc8f5f65..c7fb58417afa 100644 --- a/paimon-common/src/main/java/org/apache/paimon/data/serializer/RowCompactedSerializer.java +++ b/paimon-common/src/main/java/org/apache/paimon/data/serializer/RowCompactedSerializer.java @@ -40,6 +40,7 @@ import org.apache.paimon.types.RowKind; import org.apache.paimon.types.RowType; import org.apache.paimon.types.VectorType; +import org.apache.paimon.utils.SortUtil; import org.apache.paimon.utils.TypeCheckUtils; import org.apache.paimon.utils.VarLengthIntUtils; @@ -760,8 +761,16 @@ public int compare(MemorySlice slice1, MemorySlice slice2) { FieldReader fieldReader = fieldReaders[i]; Object o1 = fieldReader.readField(reader1, i); Object o2 = fieldReader.readField(reader2, i); - @SuppressWarnings({"unchecked", "rawtypes"}) - int comp = ((Comparable) o1).compareTo(o2); + int comp; + if (o1 instanceof byte[]) { + // BINARY / VARBINARY fields read back as byte[], which does not + // implement Comparable; order them like BinaryRow does. + comp = SortUtil.compareBinary((byte[]) o1, (byte[]) o2); + } else { + @SuppressWarnings({"unchecked", "rawtypes"}) + int comparableComp = ((Comparable) o1).compareTo(o2); + comp = comparableComp; + } if (comp != 0) { return comp; } diff --git a/paimon-common/src/test/java/org/apache/paimon/data/serializer/RowCompactedSerializerTest.java b/paimon-common/src/test/java/org/apache/paimon/data/serializer/RowCompactedSerializerTest.java index 9b55fe2261a4..833a38a6f532 100644 --- a/paimon-common/src/test/java/org/apache/paimon/data/serializer/RowCompactedSerializerTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/data/serializer/RowCompactedSerializerTest.java @@ -23,17 +23,21 @@ import org.apache.paimon.data.GenericRow; import org.apache.paimon.data.InternalRow; import org.apache.paimon.data.variant.GenericVariant; +import org.apache.paimon.memory.MemorySlice; import org.apache.paimon.types.DataType; import org.apache.paimon.types.DataTypes; import org.apache.paimon.types.RowType; import org.junit.jupiter.api.Test; +import java.util.Comparator; + import static org.apache.paimon.data.BinaryString.fromString; import static org.apache.paimon.data.serializer.InternalRowSerializerTest.createArray; import static org.apache.paimon.data.serializer.InternalRowSerializerTest.createMap; import static org.apache.paimon.data.serializer.InternalRowSerializerTest.createRow; import static org.apache.paimon.data.serializer.InternalRowSerializerTest.deepEqualsInternalRow; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** Test for {@link RowCompactedSerializer}. */ @@ -202,6 +206,44 @@ private static RowCompactedSerializer getRowSerializer() { } } + static final class BinaryFieldTest extends RowCompactedSerializerTest { + public BinaryFieldTest() { + super(getRowSerializer(), getData()); + } + + private static InternalRow[] getData() { + return new GenericRow[] { + GenericRow.of(1, new byte[] {1, 2, 3}), + GenericRow.of(1, new byte[] {1, 2, 4}), + GenericRow.of(2, new byte[] {(byte) 0xFF}) + }; + } + + private static RowCompactedSerializer getRowSerializer() { + return new RowCompactedSerializer(RowType.of(DataTypes.INT(), DataTypes.BYTES())); + } + + @Test + public void testSliceComparatorOnBinaryFields() { + RowCompactedSerializer serializer = getRowSerializer(); + Comparator comparator = serializer.createSliceComparator(); + MemorySlice small = + MemorySlice.wrap( + serializer.serializeToBytes(GenericRow.of(1, new byte[] {1, 2, 3}))); + MemorySlice large = + MemorySlice.wrap( + serializer.serializeToBytes(GenericRow.of(1, new byte[] {1, 2, 4}))); + MemorySlice unsigned = + MemorySlice.wrap( + serializer.serializeToBytes( + GenericRow.of(1, new byte[] {(byte) 0xFF}))); + assertThat(comparator.compare(small, large)).isLessThan(0); + assertThat(comparator.compare(large, small)).isGreaterThan(0); + assertThat(comparator.compare(small, small)).isEqualTo(0); + assertThat(comparator.compare(unsigned, small)).isGreaterThan(0); + } + } + static final class VectorTypesTest extends RowCompactedSerializerTest { public VectorTypesTest() { super(getRowSerializer(), getData());