From 7fd1e7b69e60311ba35be3bed1f00f62f1fc8cbc Mon Sep 17 00:00:00 2001 From: Pxl Date: Sun, 7 Apr 2024 10:53:29 +0800 Subject: [PATCH] [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 --- .../AbstractSelectMaterializedIndexRule.java | 8 - .../SelectMaterializedIndexWithAggregate.java | 216 ++++++------------ .../test_upper_alias/test_upper_alias.out | 13 ++ .../test_dup_mv_year/test_dup_mv_year.groovy | 1 - .../test_upper_alias/test_upper_alias.groovy | 69 ++++++ 5 files changed, 147 insertions(+), 160 deletions(-) create mode 100644 regression-test/data/mv_p0/test_upper_alias/test_upper_alias.out create mode 100644 regression-test/suites/mv_p0/test_upper_alias/test_upper_alias.groovy 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 da73094e5a52d9..cda43d0e329d25 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 @@ -394,9 +394,6 @@ protected SlotContext generateBaseScanExprToMvExpr(LogicalOlapScan mvPlan) { 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())))) { @@ -406,11 +403,6 @@ protected SlotContext generateBaseScanExprToMvExpr(LogicalOlapScan mvPlan) { } } 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 370d445049eaa5..0c18eabf6456d0 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 @@ -370,28 +370,16 @@ public List buildRules() { LogicalOlapScan mvPlan = createLogicalOlapScan(scan, result); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); - if (result.exprRewriteMap.isEmpty()) { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - agg.withChildren( - repeat.withAggOutputAndChild( - generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), mvPlan) - ), mvPlan)); - } else { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - new LogicalAggregate<>( - agg.getGroupByExpressions(), - replaceAggOutput( + 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(), - repeat.withAggOutputAndChild( - generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), mvPlan) - ), mvPlan)); - } + agg.isNormalized(), agg.getSourceRepeat(), + repeat.withAggOutputAndChild( + replaceRepeatOutput(repeat, result.exprRewriteMap.projectExprMap), + mvPlan)), + mvPlan)); }).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_SCAN), // filter could push down scan. @@ -423,33 +411,19 @@ public List buildRules() { LogicalOlapScan mvPlan = createLogicalOlapScan(scan, result); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); - if (result.exprRewriteMap.isEmpty()) { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - agg.withChildren( + 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(), + // 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) - )), mvPlan)); - } else { - 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(), - // 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()), + replaceRepeatOutput(repeat, result.exprRewriteMap.projectExprMap), filter.withChildren(mvPlan)) - ), mvPlan)); - } + ), mvPlan)); }).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_FILTER_SCAN), // column pruning or other projections such as alias, etc. @@ -474,37 +448,20 @@ public List buildRules() { LogicalOlapScan mvPlan = createLogicalOlapScan(scan, result); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); - if (result.exprRewriteMap.isEmpty()) { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - agg.withChildren( - repeat.withAggOutputAndChild( - generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), - project.withProjectsAndChild( - generateNewOutputsWithMvOutputs(mvPlan, project.getProjects()), - mvPlan)) - ), mvPlan)); - } else { - List newProjectList = replaceProjectList(project, - result.exprRewriteMap.projectExprMap); - LogicalProject newProject = new LogicalProject<>( - 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)); - } + List newProjectList = replaceProjectList(project, + result.exprRewriteMap.projectExprMap); + LogicalProject newProject = new LogicalProject<>( + 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(replaceRepeatOutput(repeat, + result.exprRewriteMap.projectExprMap), newProject)), + mvPlan)); }).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_PROJECT_SCAN), // filter could push down and project. @@ -538,40 +495,15 @@ public List buildRules() { LogicalOlapScan mvPlan = createLogicalOlapScan(scan, result); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); - if (result.exprRewriteMap.isEmpty()) { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - agg.withChildren( - repeat.withAggOutputAndChild( - generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), - project.withProjectsAndChild( - generateNewOutputsWithMvOutputs(mvPlan, project.getProjects()), - filter.withChildren( - 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(replaceRepeatOutput(repeat, + result.exprRewriteMap.projectExprMap), newProject)), mvPlan)); - } else { - List newProjectList = replaceProjectList(project, - result.exprRewriteMap.projectExprMap); - LogicalProject newProject = new LogicalProject<>( - generateNewOutputsWithMvOutputs(mvPlan, newProjectList), - filter.withChildren(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)); - } }).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_PROJECT_FILTER_SCAN), // filter can't push down @@ -603,40 +535,15 @@ public List buildRules() { LogicalOlapScan mvPlan = createLogicalOlapScan(scan, result); SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan); - if (result.exprRewriteMap.isEmpty()) { - return new LogicalProject<>( - generateProjectsAlias(agg.getOutputs(), slotContext), - new ReplaceExpressions(slotContext).replace( - agg.withChildren( + 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( - project.withProjectsAndChild( - generateNewOutputsWithMvOutputs(mvPlan, project.getProjects()), - mvPlan - )))), + replaceRepeatOutput(repeat, result.exprRewriteMap.projectExprMap), + filter.withChildren(newProject))), mvPlan)); - } else { - List newProjectList = replaceProjectList(project, - result.exprRewriteMap.projectExprMap); - LogicalProject newProject = new LogicalProject<>( - 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(), - repeat.withAggOutputAndChild( - generateNewOutputsWithMvOutputs(mvPlan, repeat.getOutputs()), - filter.withChildren(newProject)) - ), mvPlan)); - } }).toRule(RuleType.MATERIALIZED_INDEX_AGG_REPEAT_FILTER_PROJECT_SCAN) ); } @@ -687,22 +594,22 @@ private SelectResult select(LogicalOlapScan scan, Set requiredScanOutput, .stream() .collect(Collectors.groupingBy(index -> index.getId() == table.getBaseIndexId())); - Set 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 candidatesWithRewriting = indexesGroupByIsBaseOrNot + Set 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()) - .filter(result -> result.success).collect(Collectors.toList()); + .collect(Collectors.toSet()); + + Set 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 haveAllRequiredColumns = Streams.concat( candidatesWithoutRewriting.stream() @@ -1625,6 +1532,13 @@ private List replaceProjectList( .collect(Collectors.toList()); } + private List replaceRepeatOutput(LogicalRepeat repeat, + Map projectMap) { + return repeat.getOutputs().stream() + .map(expr -> (NamedExpression) ExpressionUtils.replaceNameExpression(expr, projectMap)) + .collect(Collectors.toList()); + } + private List nonVirtualGroupByExprs(LogicalAggregate 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 00000000000000..e0c348fcf36bba --- /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 f4a06ac04c340c..1552c5f67bee76 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 @@ -57,7 +57,6 @@ suite ("test_dup_mv_year") { } 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 00000000000000..301966baf0bf61 --- /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;" +}