[jira] [Created] (CALCITE-7558) Lattice.Measure.compareTo() collapses distinct aggregates that share the same name

["Ruiqi Dong (Jira)" <ji...@apache.org>]

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)