morrySnow commented on code in PR #35026:
URL: https://github.com/apache/doris/pull/35026#discussion_r1609805458
##########
regression-test/data/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.out:
##########
@@ -355,3 +355,51 @@
2023-12-11 1
2023-12-12 2
+-- !query33_0_before --
+o 3 9 o,o,o,o,o,o 4.666666666666667 mi 6
2
+o 4 2 o,o 4.0 yy 2 1
+
+-- !query33_0_after --
+o 3 9 o,o,o,o,o,o 4.666666666666667 mi 6
2
+o 4 2 o,o 4.0 yy 2 1
+
+-- !query33_1_before --
+o 3 9 o,o,o,o,o,o 4.666666666666667 mi 6
2
+o 4 2 o,o 4.0 yy 2 1
+
+-- !query33_1_after --
+o 3 9 o,o,o,o,o,o 4.666666666666667 mi 6
2
+o 4 2 o,o 4.0 yy 2 1
+
+-- !query34_0_before --
+o 3 \ �������������� o,o,o,o,o,o,
���������������������������� ������mi������������� ��������������
���"K?��D ZW�_-�A�Vʧ��t�E
+o 4 �������������� o,o,
���������������������������� ������yy\r������������ ��������������
���"K?��D ZW
+
+-- !query34_0_after --
+o 3 \ �������������� o,o,o,o,o,o,
���������������������������� ������mi������������� ��������������
���"K?��D ZW�_-�A�Vʧ��t�E
+o 4 �������������� o,o,
���������������������������� ������yy\r������������ ��������������
���"K?��D ZW
Review Comment:
agg state result is stable? i don't think so
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -375,35 +287,22 @@ private boolean isGroupByEquals(Pair<Plan,
LogicalAggregate<Plan>> queryTopPlanA
private static Function rollup(AggregateFunction queryAggregateFunction,
Expression queryAggregateFunctionShuttled,
Map<Expression, Expression> mvExprToMvScanExprQueryBased) {
- if (!(queryAggregateFunction instanceof CouldRollUp)) {
- return null;
- }
- Expression rollupParam = null;
- Expression viewRollupFunction = null;
- // handle simple aggregate function roll up which is not in the
AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP
- if
(mvExprToMvScanExprQueryBased.containsKey(queryAggregateFunctionShuttled)
- && AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP.keySet().stream()
- .noneMatch(aggFunction ->
aggFunction.equals(queryAggregateFunction))) {
- rollupParam =
mvExprToMvScanExprQueryBased.get(queryAggregateFunctionShuttled);
- viewRollupFunction = queryAggregateFunctionShuttled;
- } else {
- // handle complex functions roll up
- // eg: query is count(distinct param), mv sql is
bitmap_union(to_bitmap(param))
- for (Expression mvExprShuttled :
mvExprToMvScanExprQueryBased.keySet()) {
- if (!(mvExprShuttled instanceof Function)) {
+ for (Map.Entry<Expression, Expression> expressionEntry :
mvExprToMvScanExprQueryBased.entrySet()) {
+ for (AggFunctionRollUpHandler rollUpHandler : ROLL_UP_HANDLERS) {
+ Pair<Expression, Expression> mvExprToMvScanExprQueryBasedPair
= Pair.of(expressionEntry.getKey(),
+ expressionEntry.getValue());
Review Comment:
init out of inner `for` for better performance
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -72,92 +61,15 @@
*/
public abstract class AbstractMaterializedViewAggregateRule extends
AbstractMaterializedViewRule {
- protected static final Multimap<Function, Expression>
- AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP =
ArrayListMultimap.create();
+ public static List<AggFunctionRollUpHandler> ROLL_UP_HANDLERS =
+ Lists.newArrayList(DirectRollupHandler.INSTANCE,
Review Comment:
use ImmutableList
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -72,92 +61,15 @@
*/
public abstract class AbstractMaterializedViewAggregateRule extends
AbstractMaterializedViewRule {
- protected static final Multimap<Function, Expression>
- AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP =
ArrayListMultimap.create();
+ public static List<AggFunctionRollUpHandler> ROLL_UP_HANDLERS =
Review Comment:
final?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
