[CALCITE-7554] NlsString.compareTo inconsistent with equals/hashCode

[lanky228]

What

NlsString.compareTo() only compared the decoded string value, ignoring charsetName, collation, and bytesValue. This made compareTo inconsistent with equals()/hashCode(), which consider all metadata fields.

Why It Matters

This violates the Java contract: when equals() == true, compareTo() == 0 must also hold. The bug caused TreeSet and TreeMap to silently collapse distinct NlsString values that shared the same decoded string but differed in charset or collation.

Example: ("hello", "LATIN1", null) and ("hello", "UTF-8", null) are not equal per equals(), but the old compareTo() returned 0 — causing a TreeSet to keep only one of them.

Changes

NlsString.java (+32 -2):

• After comparing decoded string values (preserving the original collator branch), break ties by comparing:
  1. charsetName — Comparator.nullsFirst(String::compareTo) for null safety
  2. collation — SqlCollation does not implement Comparable, so toString() is used as proxy key
  3. bytesValue — ByteString implements Comparable<ByteString>, using Comparator.naturalOrder()
• All comparisons use Comparator.nullsFirst() for null-safe handling (fields are @Nullable)
• No new helper methods — standard library Comparator utilities only

UtilTest.java (+53):

• testNlsStringCompareToConsistency(): 5 cases verifying compareTo/equals consistency:
  • Same text, different charset → not equal, compareTo != 0
  • Same text, different collation → not equal, compareTo != 0
  • Identical values → equal, compareTo == 0
  • Both null charset → equal, compareTo == 0
  • One null, one non-null charset → not equal, compareTo != 0
• testNlsStringTreeSetRetainsDistinctValues(): 4 values with same string "hello" but different charset — TreeSet must retain all 4 (before fix: collapsed to 1)

Verification

• ./gradlew :core:compileJava — BUILD SUCCESSFUL
• ./gradlew :core:checkstyleMain — passed
• ./gradlew :core:test --tests "org.apache.calcite.util.UtilTest" — 7 tests passed

Note

This is a behavior change: TreeSet<NlsString> ordering results may differ from before. This is the intended fix — the old behavior was a bug.

Jira

CALCITE-7554

[mihaibudiu]

Please no such comments about past bugs. They will make no sense to future readers.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[lanky228]

Thanks, removed the historical comments. Kept the last line because the byte comparison step is non-obvious and needs explanation.

[mihaibudiu]

In general please make changes as new commits, so the reviewers can see what's new.