[
https://issues.apache.org/jira/browse/CALCITE-7558?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated CALCITE-7558:
------------------------------------
Labels: pull-request-available (was: )
> 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
> Priority: Major
> Labels: pull-request-available
>
> *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)
