Skip to content

Commit 135dbf7

Browse files
committed
[fix](fe) Defer multi distinct count variant check
### What problem does this PR solve? Issue Number: close #25672 Related PR: #63479 Problem Summary: MultiDistinctCount checked argument types while sync MV SQL was still unbound, so sync MV context generation skipped multi_distinct_count MVs. Defer the variant rejection to the post-rewrite legality check and keep array arguments valid. ### Release note None ### Check List (For Author) - Test: Unit Test - FE unit test: ./run-fe-ut.sh --run org.apache.doris.nereids.trees.expressions.functions.agg.CountTest - Behavior changed: No - Does this need documentation: No
1 parent b6941b9 commit 135dbf7

2 files changed

Lines changed: 37 additions & 6 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctCount.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ private MultiDistinctCount(boolean distinct, List<Expression> children) {
5757
if (super.children().size() > 1) {
5858
throw new AnalysisException("MultiDistinctCount's children size must be 1");
5959
}
60-
for (Expression argument : super.children()) {
61-
Count.checkVariantArgument(argument, "COUNT DISTINCT " + argument.toSql());
62-
}
6360
}
6461

6562
/** constructor for withChildren and reuse signature */
@@ -70,10 +67,14 @@ protected MultiDistinctCount(AggregateFunctionParams functionParams) {
7067
@Override
7168
public MultiDistinctCount withDistinctAndChildren(boolean distinct, List<Expression> children) {
7269
Preconditions.checkArgument(children.size() == 1, "MultiDistinctCount's children size must be 1");
73-
for (Expression argument : children) {
70+
return new MultiDistinctCount(getFunctionParams(false, children));
71+
}
72+
73+
@Override
74+
public void checkLegalityAfterRewrite() {
75+
for (Expression argument : getArguments()) {
7476
Count.checkVariantArgument(argument, "COUNT DISTINCT " + argument.toSql());
7577
}
76-
return new MultiDistinctCount(getFunctionParams(false, children));
7778
}
7879

7980
@Override

fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/agg/CountTest.java

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,21 @@
1717

1818
package org.apache.doris.nereids.trees.expressions.functions.agg;
1919

20+
import org.apache.doris.common.Pair;
21+
import org.apache.doris.nereids.analyzer.UnboundSlot;
2022
import org.apache.doris.nereids.exceptions.AnalysisException;
23+
import org.apache.doris.nereids.rules.exploration.mv.rollup.SingleCombinatorRollupHandler;
24+
import org.apache.doris.nereids.trees.expressions.Expression;
2125
import org.apache.doris.nereids.trees.expressions.SlotReference;
26+
import org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
27+
import org.apache.doris.nereids.trees.expressions.functions.combinator.UnionCombinator;
2228
import org.apache.doris.nereids.types.ArrayType;
2329
import org.apache.doris.nereids.types.IntegerType;
2430
import org.apache.doris.nereids.types.VariantType;
2531

32+
import com.google.common.collect.ImmutableList;
33+
import com.google.common.collect.ImmutableMap;
34+
2635
import org.junit.jupiter.api.Assertions;
2736
import org.junit.jupiter.api.Test;
2837

@@ -39,8 +48,10 @@ void testCountDistinctRejectsVariant() {
3948

4049
@Test
4150
void testMultiDistinctCountRejectsVariant() {
51+
MultiDistinctCount count = new MultiDistinctCount(SlotReference.of("v", VariantType.INSTANCE));
52+
4253
AnalysisException exception = Assertions.assertThrows(AnalysisException.class,
43-
() -> new MultiDistinctCount(SlotReference.of("v", VariantType.INSTANCE)));
54+
count::checkLegalityAfterRewrite);
4455
Assertions.assertTrue(exception.getMessage().contains("COUNT DISTINCT does not support VARIANT argument"));
4556
Assertions.assertTrue(exception.getMessage().contains("Cast the VARIANT expression"));
4657
}
@@ -50,4 +61,23 @@ void testMultiDistinctCountAllowsArray() {
5061
Assertions.assertDoesNotThrow(
5162
() -> new MultiDistinctCount(SlotReference.of("arr", ArrayType.of(IntegerType.INSTANCE))));
5263
}
64+
65+
@Test
66+
void testMultiDistinctCountAllowsUnboundArgumentBeforeLegalityCheck() {
67+
Assertions.assertDoesNotThrow(() -> new MultiDistinctCount(new UnboundSlot("kint")));
68+
}
69+
70+
@Test
71+
void testMultiDistinctCountCanRollupFromUnionState() {
72+
SlotReference kint = SlotReference.of("kint", IntegerType.INSTANCE);
73+
MultiDistinctCount queryFunction = new MultiDistinctCount(kint);
74+
StateCombinator stateCombinator = StateCombinator.create(queryFunction);
75+
UnionCombinator unionCombinator = new UnionCombinator(ImmutableList.of(stateCombinator), queryFunction);
76+
SlotReference mvSlot = SlotReference.of("__multi_distinct_count_1", unionCombinator.getDataType());
77+
Pair<Expression, Expression> mvExprToMvScanExprPair = Pair.of(unionCombinator, mvSlot);
78+
79+
Assertions.assertTrue(SingleCombinatorRollupHandler.INSTANCE.canRollup(
80+
queryFunction, queryFunction, mvExprToMvScanExprPair,
81+
ImmutableMap.of(unionCombinator, mvSlot)));
82+
}
5383
}

0 commit comments

Comments
 (0)