[fix](nereids) fix bug of SelectMaterializedIndexWithAggregate rule (#18265)

1. create a project node to adjust the output column position when a mv is selected in olap scan node
2. pass SlotReference's column info when call Alias's toSlot() method
3. should compare plan's logical properties when compare two plans after rewrite
This commit is contained in:
starocean999
2023-04-03 22:32:43 +08:00
committed by GitHub
parent 1e51af0784
commit 88c5e64c4a
8 changed files with 85 additions and 12 deletions

View File

@ -62,6 +62,7 @@ import org.apache.doris.nereids.rules.rewrite.logical.NormalizeSort;
import org.apache.doris.nereids.rules.rewrite.logical.PruneOlapScanPartition;
import org.apache.doris.nereids.rules.rewrite.logical.PruneOlapScanTablet;
import org.apache.doris.nereids.rules.rewrite.logical.PushFilterInsideJoin;
import org.apache.doris.nereids.rules.rewrite.logical.PushdownFilterThroughProject;
import org.apache.doris.nereids.rules.rewrite.logical.PushdownLimit;
import org.apache.doris.nereids.rules.rewrite.logical.ReorderJoin;
import org.apache.doris.nereids.rules.rewrite.logical.SemiJoinAggTranspose;
@ -227,18 +228,24 @@ public class NereidsRewriter extends BatchRewriteJob {
),
// TODO: I think these rules should be implementation rules, and generate alternative physical plans.
topic("Table/MV/Physical optimization",
topic("Table/Physical optimization",
topDown(
// TODO: the logical plan should not contains any phase information,
// we should refactor like AggregateStrategies, e.g. LimitStrategies,
// generate one PhysicalLimit if current distribution is gather or two
// PhysicalLimits with gather exchange
new SplitLimit(),
new PruneOlapScanPartition()
)
),
topic("MV optimization",
topDown(
new SelectMaterializedIndexWithAggregate(),
new SelectMaterializedIndexWithoutAggregate(),
new PruneOlapScanTablet(),
new PruneOlapScanPartition()
new PushdownFilterThroughProject(),
new MergeProjects(),
new PruneOlapScanTablet()
)
),

View File

@ -24,15 +24,20 @@ import org.apache.doris.catalog.OlapTable;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.rewrite.RewriteRuleFactory;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.plans.PreAggStatus;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import java.util.List;
import java.util.Set;
@ -111,7 +116,7 @@ public class SelectMaterializedIndexWithoutAggregate extends AbstractSelectMater
* @param predicatesSupplier Supplier to get pushdown predicates.
* @return Result scan node.
*/
private LogicalOlapScan select(
private LogicalPlan select(
LogicalOlapScan scan,
Supplier<Set<Slot>> requiredScanOutputSupplier,
Supplier<Set<Expression>> predicatesSupplier) {
@ -132,13 +137,17 @@ public class SelectMaterializedIndexWithoutAggregate extends AbstractSelectMater
}
if (scan.getTable().isDupKeysOrMergeOnWrite()) {
// Set pre-aggregation to `on` to keep consistency with legacy logic.
List<MaterializedIndex> candidate = scan.getTable().getVisibleIndex().stream()
List<MaterializedIndex> candidates = scan.getTable().getVisibleIndex().stream()
.filter(index -> !indexHasAggregate(index, scan))
.filter(index -> containAllRequiredColumns(index, scan,
requiredScanOutputSupplier.get()))
.collect(Collectors.toList());
return scan.withMaterializedIndexSelected(PreAggStatus.on(),
selectBestIndex(candidate, scan, predicatesSupplier.get()));
long bestIndex = selectBestIndex(candidates, scan, predicatesSupplier.get());
if (bestIndex == baseIndexId) {
return scan.withMaterializedIndexSelected(PreAggStatus.on(), bestIndex);
} else {
return createProjectForMv(scan.withMaterializedIndexSelected(PreAggStatus.on(), bestIndex));
}
} else {
final PreAggStatus preAggStatus;
if (preAggEnabledByHint(scan)) {
@ -163,8 +172,8 @@ public class SelectMaterializedIndexWithoutAggregate extends AbstractSelectMater
// `candidates` only have base index.
return scan.withMaterializedIndexSelected(preAggStatus, baseIndexId);
} else {
return scan.withMaterializedIndexSelected(preAggStatus,
selectBestIndex(candidates, scan, predicatesSupplier.get()));
return createProjectForMv(scan.withMaterializedIndexSelected(preAggStatus,
selectBestIndex(candidates, scan, predicatesSupplier.get())));
}
}
}
@ -174,4 +183,28 @@ public class SelectMaterializedIndexWithoutAggregate extends AbstractSelectMater
.stream()
.anyMatch(Column::isAggregated);
}
private LogicalProject createProjectForMv(LogicalOlapScan scan) {
Preconditions.checkArgument(scan.getSelectedIndexId() != scan.getTable().getBaseIndexId());
List<Slot> mvSlots = scan.getOutputByMvIndex(scan.getSelectedIndexId());
List<Slot> baseSlots = scan.getOutputByMvIndex(scan.getTable().getBaseIndexId());
List<Alias> aliases = Lists.newArrayList();
List<String> baseColumnNames = mvSlots.stream()
.map(slot -> org.apache.doris.analysis.CreateMaterializedViewStmt.mvColumnBreaker(slot.getName()))
.collect(Collectors.toList());
boolean isMvName = org.apache.doris.analysis.CreateMaterializedViewStmt.isMVColumn(mvSlots.get(0).getName());
for (int i = 0; i < baseColumnNames.size(); ++i) {
for (Slot slot : baseSlots) {
if (((SlotReference) slot).getColumn().get().getName()
.equals(baseColumnNames.get(i))) {
aliases.add(
new Alias(slot.getExprId(), isMvName ? mvSlots.get(i) : slot, baseColumnNames.get(i)));
break;
}
}
}
return new LogicalProject(aliases,
isMvName ? scan.withOutput(scan.getOutputByMvIndex(scan.getSelectedIndexId()))
: scan);
}
}

View File

@ -63,7 +63,10 @@ public class Alias extends NamedExpression implements UnaryExpression {
@Override
public Slot toSlot() throws UnboundException {
return new SlotReference(exprId, name, child().getDataType(), child().nullable(), qualifier);
return new SlotReference(exprId, name, child().getDataType(), child().nullable(), qualifier,
child() instanceof SlotReference
? ((SlotReference) child()).getColumn().orElse(null)
: null);
}
@Override

View File

@ -29,6 +29,7 @@ import org.apache.doris.nereids.metrics.event.CounterEvent;
import org.apache.doris.nereids.properties.LogicalProperties;
import org.apache.doris.nereids.properties.UnboundLogicalProperties;
import org.apache.doris.nereids.trees.AbstractTreeNode;
import org.apache.doris.nereids.trees.TreeNode;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.util.MutableState;
@ -194,4 +195,13 @@ public abstract class AbstractPlan extends AbstractTreeNode<Plan> implements Pla
: "";
return groupId;
}
@Override
public boolean deepEquals(TreeNode o) {
AbstractPlan that = (AbstractPlan) o;
if (Objects.equals(getLogicalProperties(), that.getLogicalProperties())) {
return super.deepEquals(o);
}
return false;
}
}