This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch 2.1-tmp
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 2092a862fc79aa758ebfcf3d607bcf190270cb03
Author: Pxl <pxl...@qq.com>
AuthorDate: Sun Apr 7 10:53:29 2024 +0800
[Bug](materialized-view) fix wrong result when salias name same with base
slot on mv (#33198)
fix wrong result when salias name same with base slot on mv
---
.../mv/AbstractSelectMaterializedIndexRule.java | 8 --
.../mv/SelectMaterializedIndexWithAggregate.java | 118 ++++++++++-----------
.../mv_p0/test_upper_alias/test_upper_alias.out | 13 +++
.../mv_p0/test_dup_mv_year/test_dup_mv_year.groovy | 1 -
.../mv_p0/test_upper_alias/test_upper_alias.groovy | 69 ++++++++++++
5 files changed, 136 insertions(+), 73 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/AbstractSelectMaterializedIndexRule.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/AbstractSelectMaterializedIndexRule.java
index e69bffb301b..23c171e0536 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/AbstractSelectMaterializedIndexRule.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/AbstractSelectMaterializedIndexRule.java
@@ -460,9 +460,6 @@ public abstract class AbstractSelectMaterializedIndexRule {
for (Slot mvSlot :
mvPlan.getOutputByIndex(mvPlan.getSelectedIndexId())) {
boolean isPushed = false;
for (Slot baseSlot : mvPlan.getOutput()) {
- if
(org.apache.doris.analysis.CreateMaterializedViewStmt.isMVColumn(mvSlot.getName()))
{
- continue;
- }
if (baseSlot.toSql().equalsIgnoreCase(
org.apache.doris.analysis.CreateMaterializedViewStmt.mvColumnBreaker(
normalizeName(mvSlot.getName())))) {
@@ -472,11 +469,6 @@ public abstract class AbstractSelectMaterializedIndexRule {
}
}
if (!isPushed) {
- if
(org.apache.doris.analysis.CreateMaterializedViewStmt.isMVColumn(mvSlot.getName()))
{
- mvNameToMvSlot.put(normalizeName(
-
org.apache.doris.analysis.CreateMaterializedViewStmt.mvColumnBreaker(mvSlot.getName())),
- mvSlot);
- }
mvNameToMvSlot.put(normalizeName(mvSlot.getName()), mvSlot);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
index b1a06e3875a..24c04937bbe 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
@@ -355,18 +355,16 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
LogicalOlapScan mvPlan = createLogicalOlapScan(scan,
result);
SlotContext slotContext =
generateBaseScanExprToMvExpr(mvPlan);
- return new LogicalProject<>(
- generateProjectsAlias(agg.getOutputs(),
slotContext),
- new ReplaceExpressions(slotContext).replace(
- new LogicalAggregate<>(
+ return new
LogicalProject<>(generateProjectsAlias(agg.getOutputs(), slotContext),
+ new
ReplaceExpressions(slotContext).replace(new LogicalAggregate<>(
agg.getGroupByExpressions(),
replaceAggOutput(
- agg, Optional.empty(),
Optional.empty(), result.exprRewriteMap),
- agg.isNormalized(),
- agg.getSourceRepeat(),
+ agg, Optional.empty(),
Optional.empty(), result.exprRewriteMap),
+ agg.isNormalized(),
agg.getSourceRepeat(),
repeat.withAggOutputAndChild(
-
generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), mvPlan)
- ), mvPlan));
+ replaceRepeatOutput(repeat,
result.exprRewriteMap.projectExprMap),
+ mvPlan)),
+ mvPlan));
}).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_SCAN),
// filter could push down scan.
@@ -400,21 +398,18 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
.map(e ->
result.exprRewriteMap.replaceAgg(e)).collect(Collectors.toSet()),
filter.getConjuncts());
- return new LogicalProject<>(
- generateProjectsAlias(agg.getOutputs(),
slotContext),
- new
ReplaceExpressions(slotContext).replace(
- new LogicalAggregate<>(
+ return new
LogicalProject<>(generateProjectsAlias(agg.getOutputs(), slotContext),
+ new
ReplaceExpressions(slotContext).replace(new LogicalAggregate<>(
agg.getGroupByExpressions(),
replaceAggOutput(agg,
Optional.empty(), Optional.empty(),
result.exprRewriteMap),
- agg.isNormalized(),
- agg.getSourceRepeat(),
+ agg.isNormalized(),
agg.getSourceRepeat(),
// Not that no need to replace
slots in the filter,
// because the slots to replace
// are value columns, which
shouldn't appear in filters.
repeat.withAggOutputAndChild(
-
generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()),
- filter.withChildren(mvPlan))
+
replaceRepeatOutput(repeat, result.exprRewriteMap.projectExprMap),
+
filter.withChildren(mvPlan))
), mvPlan));
}).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_FILTER_SCAN),
@@ -443,21 +438,17 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
List<NamedExpression> newProjectList =
replaceProjectList(project,
result.exprRewriteMap.projectExprMap);
LogicalProject<LogicalOlapScan> newProject = new
LogicalProject<>(
- generateNewOutputsWithMvOutputs(mvPlan,
newProjectList),
- mvPlan);
- return new LogicalProject<>(
- generateProjectsAlias(agg.getOutputs(),
slotContext),
+ generateNewOutputsWithMvOutputs(mvPlan,
newProjectList), mvPlan);
+
+ return new
LogicalProject<>(generateProjectsAlias(agg.getOutputs(), slotContext),
new
ReplaceExpressions(slotContext).replace(
- new LogicalAggregate<>(
- agg.getGroupByExpressions(),
- replaceAggOutput(agg,
Optional.of(project), Optional.of(newProject),
- result.exprRewriteMap),
- agg.isNormalized(),
- agg.getSourceRepeat(),
- repeat.withAggOutputAndChild(
-
generateNewOutputsWithMvOutputs(
- mvPlan,
repeat.getOutputs()), newProject)
- ), mvPlan));
+ new
LogicalAggregate<>(agg.getGroupByExpressions(),
+ replaceAggOutput(agg,
Optional.of(project), Optional.of(newProject),
+
result.exprRewriteMap),
+ agg.isNormalized(),
agg.getSourceRepeat(),
+
repeat.withAggOutputAndChild(replaceRepeatOutput(repeat,
+
result.exprRewriteMap.projectExprMap), newProject)),
+ mvPlan));
}).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_PROJECT_SCAN),
// filter could push down and project.
@@ -499,19 +490,15 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
generateNewOutputsWithMvOutputs(mvPlan,
newProjectList),
filter.withChildren(mvPlan));
- return new LogicalProject<>(
- generateProjectsAlias(agg.getOutputs(),
slotContext),
+ return new
LogicalProject<>(generateProjectsAlias(agg.getOutputs(), slotContext),
new
ReplaceExpressions(slotContext).replace(
- new LogicalAggregate<>(
- agg.getGroupByExpressions(),
- replaceAggOutput(agg,
Optional.of(project), Optional.of(newProject),
- result.exprRewriteMap),
- agg.isNormalized(),
- agg.getSourceRepeat(),
- repeat.withAggOutputAndChild(
-
generateNewOutputsWithMvOutputs(
- mvPlan,
repeat.getOutputs()), newProject)
- ), mvPlan));
+ new
LogicalAggregate<>(agg.getGroupByExpressions(),
+ replaceAggOutput(agg,
Optional.of(project), Optional.of(newProject),
+
result.exprRewriteMap),
+ agg.isNormalized(),
agg.getSourceRepeat(),
+
repeat.withAggOutputAndChild(replaceRepeatOutput(repeat,
+
result.exprRewriteMap.projectExprMap), newProject)),
+ mvPlan));
}).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_PROJECT_FILTER_SCAN),
// filter can't push down
@@ -551,19 +538,15 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
generateNewOutputsWithMvOutputs(mvPlan,
newProjectList),
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId));
- return new LogicalProject<>(
- generateProjectsAlias(agg.getOutputs(),
slotContext),
- new
ReplaceExpressions(slotContext).replace(
- new LogicalAggregate<>(
- agg.getGroupByExpressions(),
- replaceAggOutput(agg,
Optional.of(project), Optional.of(newProject),
- result.exprRewriteMap),
- agg.isNormalized(),
- agg.getSourceRepeat(),
+ return new
LogicalProject<>(generateProjectsAlias(agg.getOutputs(), slotContext),
+ new
ReplaceExpressions(slotContext).replace(new LogicalAggregate<>(
+ agg.getGroupByExpressions(),
replaceAggOutput(agg, Optional.of(project),
+ Optional.of(newProject),
result.exprRewriteMap),
+ agg.isNormalized(),
agg.getSourceRepeat(),
repeat.withAggOutputAndChild(
-
generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()),
-
filter.withChildren(newProject))
- ), mvPlan));
+
replaceRepeatOutput(repeat, result.exprRewriteMap.projectExprMap),
+
filter.withChildren(newProject))),
+ mvPlan));
}).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_FILTER_PROJECT_SCAN)
);
}
@@ -614,22 +597,22 @@ public class SelectMaterializedIndexWithAggregate extends
AbstractSelectMaterial
.stream()
.collect(Collectors.groupingBy(index -> index.getId() ==
table.getBaseIndexId()));
- Set<MaterializedIndex> candidatesWithoutRewriting =
indexesGroupByIsBaseOrNot
- .getOrDefault(false, ImmutableList.of()).stream()
- .filter(index -> preAggEnabledByHint(scan)
- || checkPreAggStatus(scan, index.getId(), predicates,
aggregateFunctions, groupingExprs).isOn())
- .collect(Collectors.toSet());
-
// try to rewrite bitmap, hll by materialized index columns.
- List<AggRewriteResult> candidatesWithRewriting =
indexesGroupByIsBaseOrNot
+ Set<AggRewriteResult> candidatesWithRewriting =
indexesGroupByIsBaseOrNot
.getOrDefault(false, ImmutableList.of()).stream()
- .filter(index -> !candidatesWithoutRewriting.contains(index))
.map(index -> rewriteAgg(index, scan,
nonVirtualRequiredScanOutput, predicates, aggregateFunctions,
groupingExprs))
.filter(aggRewriteResult -> checkPreAggStatus(scan,
aggRewriteResult.index.getId(), predicates,
// check pre-agg status of aggregate function that
couldn't rewrite.
aggFuncsDiff(aggregateFunctions, aggRewriteResult),
groupingExprs).isOn())
- .collect(Collectors.toList());
+ .collect(Collectors.toSet());
+
+ Set<MaterializedIndex> candidatesWithoutRewriting =
indexesGroupByIsBaseOrNot
+ .getOrDefault(false, ImmutableList.of()).stream()
+ .filter(index -> !candidatesWithRewriting.contains(index))
+ .filter(index -> preAggEnabledByHint(scan)
+ || checkPreAggStatus(scan, index.getId(), predicates,
aggregateFunctions, groupingExprs).isOn())
+ .collect(Collectors.toSet());
List<MaterializedIndex> haveAllRequiredColumns = Streams.concat(
candidatesWithoutRewriting.stream()
@@ -1595,6 +1578,13 @@ public class SelectMaterializedIndexWithAggregate
extends AbstractSelectMaterial
.collect(Collectors.toList());
}
+ private List<NamedExpression> replaceRepeatOutput(LogicalRepeat<? extends
Plan> repeat,
+ Map<Expression, Expression> projectMap) {
+ return repeat.getOutputs().stream()
+ .map(expr -> (NamedExpression)
ExpressionUtils.replaceNameExpression(expr, projectMap))
+ .collect(Collectors.toList());
+ }
+
private List<Expression> nonVirtualGroupByExprs(LogicalAggregate<? extends
Plan> agg) {
return agg.getGroupByExpressions().stream()
.filter(expr -> !(expr instanceof VirtualSlotReference))
diff --git a/regression-test/data/mv_p0/test_upper_alias/test_upper_alias.out
b/regression-test/data/mv_p0/test_upper_alias/test_upper_alias.out
new file mode 100644
index 00000000000..e0c348fcf36
--- /dev/null
+++ b/regression-test/data/mv_p0/test_upper_alias/test_upper_alias.out
@@ -0,0 +1,13 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !select_mv --
+XXX
+YYY
+
+-- !select_mv --
+XXX
+YYY
+
+-- !select_mv --
+wfsdf
+wfsdf
+
diff --git
a/regression-test/suites/mv_p0/test_dup_mv_year/test_dup_mv_year.groovy
b/regression-test/suites/mv_p0/test_dup_mv_year/test_dup_mv_year.groovy
index 2e41fb0923d..059439418e9 100644
--- a/regression-test/suites/mv_p0/test_dup_mv_year/test_dup_mv_year.groovy
+++ b/regression-test/suites/mv_p0/test_dup_mv_year/test_dup_mv_year.groovy
@@ -47,7 +47,6 @@ suite ("test_dup_mv_year") {
createMV "create materialized view k13y as select k1,year(k3) from
d_table;"
sql "insert into d_table select 4,'2033-12-31','2033-12-31 01:02:03';"
- Thread.sleep(1000)
qt_select_star "select * from d_table order by k1;"
diff --git
a/regression-test/suites/mv_p0/test_upper_alias/test_upper_alias.groovy
b/regression-test/suites/mv_p0/test_upper_alias/test_upper_alias.groovy
new file mode 100644
index 00000000000..301966baf0b
--- /dev/null
+++ b/regression-test/suites/mv_p0/test_upper_alias/test_upper_alias.groovy
@@ -0,0 +1,69 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.codehaus.groovy.runtime.IOGroovyMethods
+
+suite ("test_upper_alias") {
+ sql """set enable_nereids_planner=true"""
+ sql """SET enable_fallback_to_original_planner=false"""
+ sql """ drop table if exists test_0401;"""
+
+ sql """
+ CREATE TABLE test_0401 (
+ `d_b` varchar(128) NULL,
+ `d_a` varchar(128) NULL,
+ `amt_b0` double NULL
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`d_b`)
+ DISTRIBUTED BY HASH(`d_b`) BUCKETS 3
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1"
+ );
+ """
+
+ sql """insert into test_0401 values('xxx', 'wfsdf', 9.30 );"""
+
+ createMV ("""
+ create materialized view test_0401_mv as
+ select d_b, sum(amt_b0) as amt_b0 from test_0401 group by d_b;
+ """)
+
+ createMV ("""
+ create materialized view test_0401_mv2 as
+ select d_a,d_b from test_0401;
+ """)
+
+ sql """insert into test_0401 values('yyy', 'wfsdf', 91.310 );"""
+
+ explain {
+ sql("SELECT upper(d_b) AS d_b FROM test_0401 GROUP BY upper(d_b) order
by 1;")
+ contains "(test_0401_mv)"
+ }
+ qt_select_mv "SELECT upper(d_b) AS d_b FROM test_0401 GROUP BY upper(d_b)
order by 1;"
+
+ explain {
+ sql("SELECT upper(d_b) AS d_bb FROM test_0401 GROUP BY upper(d_b)
order by 1;")
+ contains "(test_0401_mv)"
+ }
+ qt_select_mv "SELECT upper(d_b) AS d_bb FROM test_0401 GROUP BY upper(d_b)
order by 1;"
+
+ explain {
+ sql("SELECT d_a AS d_b FROM test_0401 order by 1;")
+ contains "(test_0401_mv2)"
+ }
+ qt_select_mv "SELECT d_a AS d_b FROM test_0401 order by 1;"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
