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 6764c21bc0 [CALCITE-7555] Sarg.compareTo() collapses semantically
different search arguments that have different nullAs
6764c21bc0 is described below
commit 6764c21bc048511ac077f1f8ac97f1c0e6234983
Author: Anas Khan <[email protected]>
AuthorDate: Fri Jul 3 10:31:54 2026 +0530
[CALCITE-7555] Sarg.compareTo() collapses semantically different search
arguments that have different nullAs
Sarg.equals and Sarg.hashCode both account for nullAs, but compareTo
ordered only by the range set. Two Sargs over the same ranges but with
different null semantics (for example RexUnknownAs.UNKNOWN versus
RexUnknownAs.FALSE) were therefore unequal as objects yet compared as 0,
violating the Comparable contract. A sorted collection keyed on Sarg,
such as a TreeSet or TreeMap, could silently drop one of two
semantically distinct search arguments.
Tie-break on nullAs after the range-set comparison so that compareTo
agrees with equals.
---
.../main/java/org/apache/calcite/util/Sarg.java | 10 ++++++++-
.../org/apache/calcite/rex/RexProgramTest.java | 26 ++++++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/calcite/util/Sarg.java
b/core/src/main/java/org/apache/calcite/util/Sarg.java
index fc13735717..89e4c6e9a1 100644
--- a/core/src/main/java/org/apache/calcite/util/Sarg.java
+++ b/core/src/main/java/org/apache/calcite/util/Sarg.java
@@ -199,7 +199,15 @@ public StringBuilder printTo(StringBuilder sb,
}
@Override public int compareTo(Sarg<C> o) {
- return RangeSets.compare(rangeSet, o.rangeSet);
+ int c = RangeSets.compare(rangeSet, o.rangeSet);
+ if (c != 0) {
+ return c;
+ }
+ // Tie-break on nullAs so that compareTo is consistent with equals and
+ // hashCode, which both account for nullAs. Two Sargs over the same ranges
+ // but with different null semantics must not compare as equal, otherwise a
+ // sorted collection keyed on Sarg would silently drop one of them.
+ return Integer.compare(nullAs.ordinal(), o.nullAs.ordinal());
}
@Override public int hashCode() {
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
index e15af17d81..60fd60a837 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
@@ -68,6 +68,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
+import java.util.TreeSet;
import java.util.function.Supplier;
import static org.apache.calcite.test.Matchers.isRangeSet;
@@ -4126,6 +4127,31 @@ private void checkSarg(String message, Sarg sarg,
assertThat(sarg.isComplementedPoints(), is(true));
}
+ /** Unit test for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7555";>[CALCITE-7555]
+ * {@code Sarg.compareTo} collapses semantically different search arguments
+ * that have different {@code nullAs}</a>.
+ *
+ * <p>{@link Sarg#equals} and {@link Sarg#hashCode} account for
+ * {@link Sarg#nullAs}, but {@link Sarg#compareTo} used to order only by the
+ * range set, so two Sargs over the same ranges but with different null
+ * semantics were unequal yet compared as {@code 0}. A sorted collection
keyed
+ * on Sarg would then silently drop one of them. */
+ @Test void testSargCompareToIsConsistentWithEquals() {
+ final ImmutableRangeSet<Integer> singleton =
+ ImmutableRangeSet.of(Range.singleton(1));
+ final Sarg<Integer> unknown = Sarg.of(RexUnknownAs.UNKNOWN, singleton);
+ final Sarg<Integer> falseSarg = Sarg.of(RexUnknownAs.FALSE, singleton);
+
+ assertFalse(unknown.equals(falseSarg));
+ assertThat(unknown.compareTo(falseSarg) == 0, is(false));
+
+ final TreeSet<Sarg<Integer>> values = new TreeSet<>();
+ values.add(unknown);
+ values.add(falseSarg);
+ assertThat(values, hasSize(2));
+ }
+
@Test void testInterpreter() {
assertThat(eval(trueLiteral), is(true));
assertThat(eval(nullInt), is(NullSentinel.INSTANCE));
