[
https://issues.apache.org/jira/browse/CALCITE-7555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruiqi Dong updated CALCITE-7555:
--------------------------------
Description:
*Summary*
Sarg.compareTo() compares only the underlying rangeSet, while equals() and
hashCode() also include nullAs. This means two search arguments over the same
ranges but with different null semantics compare as equal in the natural
ordering even though they are not equal as objects. Any sorted collection keyed
on Sarg can therefore silently merge semantically different values.
*Affected code*
File: core/src/main/java/org/apache/calcite/util/Sarg.java
{code:java}
@Override public int compareTo(Sarg<C> o) {
return RangeSets.compare(rangeSet, o.rangeSet);
}
@Override public int hashCode() {
return RangeSets.hashCode(rangeSet) * 31 + nullAs.ordinal();
}
@Override public boolean equals(@Nullable Object o) {
return o == this
|| o instanceof Sarg
&& nullAs == ((Sarg) o).nullAs
&& rangeSet.equals(((Sarg) o).rangeSet);
} {code}
*Reproducer*
Add the following test to
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
{code:java}
@Test void testSargNaturalOrderingKeepsDistinctNullSemantics() {
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));
final TreeSet<Sarg<Integer>> values = new TreeSet<>();
values.add(unknown);
values.add(falseSarg);
assertThat(values, hasSize(2));
} {code}
Run:
{code:java}
./gradlew :core:test \ --tests
org.apache.calcite.util.MtClawCalciteBugTest.testSargNaturalOrderingKeepsDistinctNullSemantics
{code}
Observed behavior:
The test fails because the natural ordering collapses the two distinct Sarg
values
{code:java}
Expected: a collection with size <2>
but: collection size was <1> {code}
Expected behavior:
nullAs changes the meaning of a search argument. Two Sarg instances that differ
in nullAs should not compare as the same element in sorted collections.
This is a semantic bug, not just a Comparable cleanliness issue. A Sarg is more
than its range set. nullAs changes SQL three-valued semantics, but compareTo()
ignores it completely and allows sorted collections to merge semantically
different search arguments.
was:
*Summary*
Sarg.compareTo() compares only the underlying rangeSet, while equals() and
hashCode() also include nullAs. This means two search arguments over the same
ranges but with different null semantics compare as equal in the natural
ordering even though they are not equal as objects. Any sorted collection keyed
on Sarg can therefore silently merge semantically different values.
*Affected code*
File: core/src/main/java/org/apache/calcite/util/Sarg.java
{code:java}
@Override public int compareTo(Sarg<C> o) {
return RangeSets.compare(rangeSet, o.rangeSet);
}
@Override public int hashCode() {
return RangeSets.hashCode(rangeSet) * 31 + nullAs.ordinal();
}
@Override public boolean equals(@Nullable Object o) {
return o == this
|| o instanceof Sarg
&& nullAs == ((Sarg) o).nullAs
&& rangeSet.equals(((Sarg) o).rangeSet);
} {code}
*Reproducer*
Add the following test to
core/src/test/java/org/apache/calcite/util/MtClawCalciteBugTest.jav
{code:java}
@Test void testSargNaturalOrderingKeepsDistinctNullSemantics() {
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));
final TreeSet<Sarg<Integer>> values = new TreeSet<>();
values.add(unknown);
values.add(falseSarg);
assertThat(values, hasSize(2));
} {code}
Run:
{code:java}
./gradlew :core:test \
--tests
org.apache.calcite.util.MtClawCalciteBugTest.testSargNaturalOrderingKeepsDistinctNullSemantics
{code}
Observed behavior:
The test fails because the natural ordering collapses the two distinct Sarg
values
{code:java}
Expected: a collection with size <2>
but: collection size was <1> {code}
Expected behavior:
nullAs changes the meaning of a search argument. Two Sarg instances that differ
in nullAs should not compare as the same element in sorted collections.
This is a semantic bug, not just a Comparable cleanliness issue. A Sarg is more
than its range set. nullAs changes SQL three-valued semantics, but compareTo()
ignores it completely and allows sorted collections to merge semantically
different search arguments.
> Sarg.compareTo() collapses semantically different search arguments that have
> different nullAs
> ---------------------------------------------------------------------------------------------
>
> Key: CALCITE-7555
> URL: https://issues.apache.org/jira/browse/CALCITE-7555
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.41.0
> Reporter: Ruiqi Dong
> Priority: Major
>
> *Summary*
> Sarg.compareTo() compares only the underlying rangeSet, while equals() and
> hashCode() also include nullAs. This means two search arguments over the same
> ranges but with different null semantics compare as equal in the natural
> ordering even though they are not equal as objects. Any sorted collection
> keyed on Sarg can therefore silently merge semantically different values.
>
> *Affected code*
> File: core/src/main/java/org/apache/calcite/util/Sarg.java
> {code:java}
> @Override public int compareTo(Sarg<C> o) {
> return RangeSets.compare(rangeSet, o.rangeSet);
> }
> @Override public int hashCode() {
> return RangeSets.hashCode(rangeSet) * 31 + nullAs.ordinal();
> }
> @Override public boolean equals(@Nullable Object o) {
> return o == this
> || o instanceof Sarg
> && nullAs == ((Sarg) o).nullAs
> && rangeSet.equals(((Sarg) o).rangeSet);
> } {code}
>
> *Reproducer*
> Add the following test to
> core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
> {code:java}
> @Test void testSargNaturalOrderingKeepsDistinctNullSemantics() {
> 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));
> final TreeSet<Sarg<Integer>> values = new TreeSet<>();
> values.add(unknown);
> values.add(falseSarg);
> assertThat(values, hasSize(2));
> } {code}
> Run:
> {code:java}
> ./gradlew :core:test \ --tests
> org.apache.calcite.util.MtClawCalciteBugTest.testSargNaturalOrderingKeepsDistinctNullSemantics
> {code}
> Observed behavior:
> The test fails because the natural ordering collapses the two distinct Sarg
> values
> {code:java}
> Expected: a collection with size <2>
> but: collection size was <1> {code}
> Expected behavior:
> nullAs changes the meaning of a search argument. Two Sarg instances that
> differ in nullAs should not compare as the same element in sorted collections.
>
> This is a semantic bug, not just a Comparable cleanliness issue. A Sarg is
> more than its range set. nullAs changes SQL three-valued semantics, but
> compareTo() ignores it completely and allows sorted collections to merge
> semantically different search arguments.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
