From 894d5bf2a281106c52230567fd4f0d33748f0bf9 Mon Sep 17 00:00:00 2001 From: amalsaidov Date: Sat, 14 Nov 2026 01:00:59 +0700 Subject: [PATCH] Fix Binary.compareTo to order bytes as unsigned in UDF and Pipe APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Binary type in udf-api and pipe-api compared bytes with a signed subtraction (this.values[i] - other.values[i]). Because a Java byte is signed, any byte with the high bit set (>= 0x80) was treated as negative and sorted before ASCII bytes. That ordering disagrees with TsFile's own Binary.compareTo and with java.lang.String ordering, both of which treat bytes as unsigned. So a value containing non-ASCII UTF-8 (for example "é", encoded as 0xC3 0xA9) or binary data with high bytes sorted incorrectly relative to plain ASCII values when a UDF or pipe plugin compared Binary values. Compare each byte as an unsigned value (& 0xFF) so the result matches the lexicographic ordering used everywhere else. Added unit tests in both modules covering unsigned ordering, agreement with String ordering, ASCII ordering, and prefix/length handling. --- .../apache/iotdb/pipe/api/type/Binary.java | 5 +- .../iotdb/pipe/api/type/BinaryTest.java | 61 +++++++++++++++++++ iotdb-api/udf-api/pom.xml | 5 ++ .../org/apache/iotdb/udf/api/type/Binary.java | 5 +- .../apache/iotdb/udf/api/type/BinaryTest.java | 61 +++++++++++++++++++ 5 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 iotdb-api/pipe-api/src/test/java/org/apache/iotdb/pipe/api/type/BinaryTest.java create mode 100644 iotdb-api/udf-api/src/test/java/org/apache/iotdb/udf/api/type/BinaryTest.java diff --git a/iotdb-api/pipe-api/src/main/java/org/apache/iotdb/pipe/api/type/Binary.java b/iotdb-api/pipe-api/src/main/java/org/apache/iotdb/pipe/api/type/Binary.java index 3a878f58a62e7..3ea06382a15d0 100644 --- a/iotdb-api/pipe-api/src/main/java/org/apache/iotdb/pipe/api/type/Binary.java +++ b/iotdb-api/pipe-api/src/main/java/org/apache/iotdb/pipe/api/type/Binary.java @@ -72,7 +72,10 @@ public int compareTo(Binary other) { i++; continue; } - return this.values[i] - other.values[i]; + // Compare the bytes as unsigned values so that the ordering matches the lexicographic + // (UTF-8 / memcmp) ordering used by TsFile and java.lang.String. A plain byte subtraction + // would treat bytes >= 0x80 as negative and sort them before ASCII bytes. + return (this.values[i] & 0xFF) - (other.values[i] & 0xFF); } return getLength() - other.getLength(); } diff --git a/iotdb-api/pipe-api/src/test/java/org/apache/iotdb/pipe/api/type/BinaryTest.java b/iotdb-api/pipe-api/src/test/java/org/apache/iotdb/pipe/api/type/BinaryTest.java new file mode 100644 index 0000000000000..a8266ce0174fc --- /dev/null +++ b/iotdb-api/pipe-api/src/test/java/org/apache/iotdb/pipe/api/type/BinaryTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.pipe.api.type; + +import org.junit.Assert; +import org.junit.Test; + +public class BinaryTest { + + @Test + public void compareToTreatsBytesAsUnsigned() { + // 0x41 ('A') is 65, 0x80 is 128 when read as an unsigned byte. The one with the larger + // unsigned value must sort after the smaller one. + final Binary low = new Binary(new byte[] {(byte) 0x41}); + final Binary high = new Binary(new byte[] {(byte) 0x80}); + + Assert.assertTrue(low.compareTo(high) < 0); + Assert.assertTrue(high.compareTo(low) > 0); + } + + @Test + public void compareToMatchesStringOrderingForNonAsciiText() { + // "é" is encoded as the two bytes 0xC3 0xA9 in UTF-8, so its first byte (195 unsigned) is + // greater than 'A' (65). The ordering must agree with java.lang.String ordering. + final Binary a = new Binary("A"); + final Binary e = new Binary("é"); + + Assert.assertEquals("A".compareTo("é") < 0, a.compareTo(e) < 0); + Assert.assertTrue(a.compareTo(e) < 0); + } + + @Test + public void compareToKeepsAsciiOrdering() { + Assert.assertTrue(new Binary("A").compareTo(new Binary("B")) < 0); + Assert.assertTrue(new Binary("abc").compareTo(new Binary("abd")) < 0); + Assert.assertEquals(0, new Binary("abc").compareTo(new Binary("abc"))); + } + + @Test + public void compareToUsesLengthWhenOneIsPrefixOfTheOther() { + Assert.assertTrue(new Binary("ab").compareTo(new Binary("abc")) < 0); + Assert.assertTrue(new Binary("abc").compareTo(new Binary("ab")) > 0); + } +} diff --git a/iotdb-api/udf-api/pom.xml b/iotdb-api/udf-api/pom.xml index e90dd26be6c47..0f5177305d372 100644 --- a/iotdb-api/udf-api/pom.xml +++ b/iotdb-api/udf-api/pom.xml @@ -34,6 +34,11 @@ common ${tsfile.version} + + junit + junit + test + diff --git a/iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/type/Binary.java b/iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/type/Binary.java index e2b3ef857016a..7f3e10d61b5e7 100644 --- a/iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/type/Binary.java +++ b/iotdb-api/udf-api/src/main/java/org/apache/iotdb/udf/api/type/Binary.java @@ -72,7 +72,10 @@ public int compareTo(Binary other) { i++; continue; } - return this.values[i] - other.values[i]; + // Compare the bytes as unsigned values so that the ordering matches the lexicographic + // (UTF-8 / memcmp) ordering used by TsFile and java.lang.String. A plain byte subtraction + // would treat bytes >= 0x80 as negative and sort them before ASCII bytes. + return (this.values[i] & 0xFF) - (other.values[i] & 0xFF); } return getLength() - other.getLength(); } diff --git a/iotdb-api/udf-api/src/test/java/org/apache/iotdb/udf/api/type/BinaryTest.java b/iotdb-api/udf-api/src/test/java/org/apache/iotdb/udf/api/type/BinaryTest.java new file mode 100644 index 0000000000000..5309088838a22 --- /dev/null +++ b/iotdb-api/udf-api/src/test/java/org/apache/iotdb/udf/api/type/BinaryTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.udf.api.type; + +import org.junit.Assert; +import org.junit.Test; + +public class BinaryTest { + + @Test + public void compareToTreatsBytesAsUnsigned() { + // 0x41 ('A') is 65, 0x80 is 128 when read as an unsigned byte. The one with the larger + // unsigned value must sort after the smaller one. + final Binary low = new Binary(new byte[] {(byte) 0x41}); + final Binary high = new Binary(new byte[] {(byte) 0x80}); + + Assert.assertTrue(low.compareTo(high) < 0); + Assert.assertTrue(high.compareTo(low) > 0); + } + + @Test + public void compareToMatchesStringOrderingForNonAsciiText() { + // "é" is encoded as the two bytes 0xC3 0xA9 in UTF-8, so its first byte (195 unsigned) is + // greater than 'A' (65). The ordering must agree with java.lang.String ordering. + final Binary a = new Binary("A"); + final Binary e = new Binary("é"); + + Assert.assertEquals("A".compareTo("é") < 0, a.compareTo(e) < 0); + Assert.assertTrue(a.compareTo(e) < 0); + } + + @Test + public void compareToKeepsAsciiOrdering() { + Assert.assertTrue(new Binary("A").compareTo(new Binary("B")) < 0); + Assert.assertTrue(new Binary("abc").compareTo(new Binary("abd")) < 0); + Assert.assertEquals(0, new Binary("abc").compareTo(new Binary("abc"))); + } + + @Test + public void compareToUsesLengthWhenOneIsPrefixOfTheOther() { + Assert.assertTrue(new Binary("ab").compareTo(new Binary("abc")) < 0); + Assert.assertTrue(new Binary("abc").compareTo(new Binary("ab")) > 0); + } +}