Ruiqi Dong created CALCITE-7558:
-----------------------------------
Summary: Lattice.Measure.compareTo() collapses distinct aggregates
that share the same name
Key: CALCITE-7558
URL: https://issues.apache.org/jira/browse/CALCITE-7558
Project: Calcite
Issue Type: Bug
Components: core
Affects Versions: 1.41.0
Reporter: Ruiqi Dong
*Summary*
Lattice.Measure exposes a natural ordering that can silently drop distinct
measures from the lattice builder's TreeSet. compareTo() orders measures by:
# argument ordinals
# agg.getName()
# distinct
but equals() uses agg.equals(...), which is stricter than name equality. As a
result, two measures with the same argument list, the same distinct flag, and
the same aggregate name can compare as equal in the natural ordering even when
they are not equal as objects.
*Affected code*
File: core/src/main/java/org/apache/calcite/materialize/Lattice.java
{code:java}
@Override public int compareTo(Measure measure) {
int c = compare(args, measure.args);
if (c == 0) {
c = agg.getName().compareTo(measure.agg.getName());
if (c == 0) {
c = Boolean.compare(distinct, measure.distinct);
}
}
return c;
}
@Override public boolean equals(@Nullable Object obj) {
return obj == this
|| obj instanceof Measure
&& this.agg.equals(((Measure) obj).agg)
&& this.args.equals(((Measure) obj).args)
&& this.distinct == ((Measure) obj).distinct;
} {code}
The lattice builder stores measures in a sorted set:
{code:java}
private final NavigableSet<Measure> defaultMeasureSet =
new TreeSet<>();
public boolean addMeasure(Measure measure) {
return defaultMeasureSet.add(measure);
} {code}
*Reproducer*Add the following test to
core/src/test/java/org/apache/calcite/materialize/LatticeSuggesterTest.java
{code:java}
@Test void testMeasureNaturalOrderingKeepsDistinctAggregatorsWithSameName() {
final SqlAggFunction builtInSum = SqlStdOperatorTable.SUM;
final SqlAggFunction customSum =
new SqlAggFunction("SUM", null, SqlKind.OTHER_FUNCTION,
ReturnTypes.ARG0_NULLABLE, null, OperandTypes.NUMERIC,
SqlFunctionCategory.USER_DEFINED_FUNCTION, false, false,
Optionality.FORBIDDEN) {
};
final Measure builtInMeasure =
new Measure(builtInSum, false, "SUM", ImmutableList.of());
final Measure customMeasure =
new Measure(customSum, false, "SUM", ImmutableList.of());
assertFalse(builtInMeasure.equals(customMeasure));
final TreeSet<Measure> measures = new TreeSet<>();
measures.add(builtInMeasure);
measures.add(customMeasure);
assertThat(measures, hasSize(2));
}{code}
Run:
{code:java}
./gradlew :core:test \
--tests
org.apache.calcite.materialize.LatticeSuggesterTest.testMeasureNaturalOrderingKeepsDistinctAggregatorsWithSameName
{code}
Observed behavior:
The second distinct measure is dropped by the natural ordering
{code:java}
Expected: a collection with size <2>
but: collection size was <1> {code}
Expected behavior:
If two Lattice.Measure instances are not equal as values, the natural ordering
should not collapse them into one sorted-set entry.
The lattice builder actually uses TreeSet<Measure> to deduplicate measures, so
inconsistent ordering can silently discard a distinct aggregate definition
before the lattice is built.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
