This is an automated email from the ASF dual-hosted git repository.
mihaibudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 8c33d54d76 [CALCITE-7554] NlsString.compareTo inconsistent with
equals/hashCode
8c33d54d76 is described below
commit 8c33d54d76767493f1a22d71cb8c25da32be05d2
Author: liuzhengri <[email protected]>
AuthorDate: Fri Jul 31 22:57:15 2026 +0800
[CALCITE-7554] NlsString.compareTo inconsistent with equals/hashCode
compareTo() only compared the decoded string, ignoring charset,
collation, and byte representation. This broke the compareTo/equals
contract: two NlsStrings with same text but different charset would
compare as equal, causing TreeSet to silently collapse them.
Fix: after string comparison, compare charsetName, collation
(via toString()), and bytesValue. Uses Comparator.nullsFirst
for null-safe comparison.
---
.../java/org/apache/calcite/util/NlsString.java | 29 +++++++++++-
.../java/org/apache/calcite/util/UtilTest.java | 53 ++++++++++++++++++++++
2 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/util/NlsString.java
b/core/src/main/java/org/apache/calcite/util/NlsString.java
index b98984bf53..b000411864 100644
--- a/core/src/main/java/org/apache/calcite/util/NlsString.java
+++ b/core/src/main/java/org/apache/calcite/util/NlsString.java
@@ -38,6 +38,7 @@
import java.nio.charset.CharsetDecoder;
import java.nio.charset.IllegalCharsetNameException;
import java.nio.charset.UnsupportedCharsetException;
+import java.util.Comparator;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
@@ -185,10 +186,34 @@ private NlsString(@Nullable String stringValue, @Nullable
ByteString bytesValue,
}
@Override public int compareTo(NlsString other) {
+ int cmp;
if (collation != null && collation.getCollator() != null) {
- return collation.getCollator().compare(getValue(), other.getValue());
+ cmp = collation.getCollator().compare(getValue(), other.getValue());
+ } else {
+ cmp = getValue().compareTo(other.getValue());
+ }
+ if (cmp != 0) {
+ return cmp;
+ }
+ cmp =
+ Objects.compare(
+ charsetName, other.charsetName,
+ Comparator.nullsFirst(String::compareTo));
+ if (cmp != 0) {
+ return cmp;
+ }
+ cmp =
+ Objects.compare(
+ collation, other.collation,
+ Comparator.nullsFirst(
+ Comparator.comparing(Object::toString)));
+ if (cmp != 0) {
+ return cmp;
}
- return getValue().compareTo(other.getValue());
+ // Ensures compareTo==0 <-> equals==true
+ return Objects.compare(
+ bytesValue, other.bytesValue,
+ Comparator.nullsFirst(Comparator.naturalOrder()));
}
@Pure
diff --git a/core/src/test/java/org/apache/calcite/util/UtilTest.java
b/core/src/test/java/org/apache/calcite/util/UtilTest.java
index 7a6df9a5a0..c7cf33a66c 100644
--- a/core/src/test/java/org/apache/calcite/util/UtilTest.java
+++ b/core/src/test/java/org/apache/calcite/util/UtilTest.java
@@ -3002,6 +3002,59 @@ private void checkNameMultimap(String s,
NameMultimap<Integer> map) {
assertThat(s2, hasToString(s.toString()));
}
+ /** Tests that {@link NlsString#compareTo} is consistent with
+ * {@link NlsString#equals}: {@code x.compareTo(y) == 0} iff
+ * {@code x.equals(y)} for values that differ in charset or collation. */
+ @Test void testNlsStringCompareToConsistency() {
+ // ("hello","LATIN1",null) vs ("hello","UTF-8",null) -> equals=false,
compareTo!=0
+ final NlsString latin1 = new NlsString("hello", "LATIN1", null);
+ final NlsString utf8 = new NlsString("hello", "UTF-8", null);
+ assertThat(latin1.equals(utf8), is(false));
+ assertThat(latin1.compareTo(utf8), not(equalTo(0)));
+
+ // ("hello","UTF-8",null) vs ("hello","UTF-8",IMPLICIT) -> equals=false,
compareTo!=0
+ final NlsString noColl = new NlsString("hello", "UTF-8", null);
+ final NlsString withColl =
+ new NlsString("hello", "UTF-8", SqlCollation.IMPLICIT);
+ assertThat(noColl.equals(withColl), is(false));
+ assertThat(noColl.compareTo(withColl), not(equalTo(0)));
+
+ // ("hello","UTF-8",IMPLICIT) vs ("hello","UTF-8",IMPLICIT) ->
equals=true, compareTo==0
+ final NlsString a = new NlsString("hello", "UTF-8", SqlCollation.IMPLICIT);
+ final NlsString b = new NlsString("hello", "UTF-8", SqlCollation.IMPLICIT);
+ assertThat(a.equals(b), is(true));
+ assertThat(a.compareTo(b), is(0));
+
+ // ("hello",null,null) vs ("hello",null,null) -> equals=true, compareTo==0
+ final NlsString n1 = new NlsString("hello", null, null);
+ final NlsString n2 = new NlsString("hello", null, null);
+ assertThat(n1.equals(n2), is(true));
+ assertThat(n1.compareTo(n2), is(0));
+
+ // ("hello",null,null) vs ("hello","UTF-8",null) -> equals=false,
compareTo!=0
+ final NlsString n3 = new NlsString("hello", null, null);
+ final NlsString n4 = new NlsString("hello", "UTF-8", null);
+ assertThat(n3.equals(n4), is(false));
+ assertThat(n3.compareTo(n4), not(equalTo(0)));
+ }
+
+ @Test void testNlsStringTreeSetRetainsDistinctValues() {
+ // 4 values, same string "hello", different charset: TreeSet must keep all
4
+ // Before fix: compareTo ignored charset, TreeSet collapsed them into 1
+ final NlsString s1 = new NlsString("hello", "LATIN1", null);
+ final NlsString s2 = new NlsString("hello", "UTF-8", null);
+ final NlsString s3 = new NlsString("hello", "UTF-16", null);
+ final NlsString s4 = new NlsString("hello", null, null);
+
+ final SortedSet<NlsString> set = new TreeSet<>(Arrays.asList(s1, s2, s3,
s4));
+ assertThat(set, hasSize(4));
+
+ // Add exact duplicate of s1: set size unchanged
+ final NlsString s1dup = new NlsString("hello", "LATIN1", null);
+ set.add(s1dup);
+ assertThat(set, hasSize(4));
+ }
+
@Test void testCollationEncoding() {
SqlCollation collation =
new SqlCollation(
