Skip to content

Commit

Permalink
[fix](compile) fe compile failed when generate doc and FE UT failed (#…
Browse files Browse the repository at this point in the history 
…26164)

1. FE could not compile because below error. Intro by PR #25933
```
[INFO] --- exec:3.1.0:java (doc) @ fe-core ---
...
Failed to generate doc for ignoreRuntimeFilterIds
```

2. fix UT bugs intro by below PRs
> - #25951
> - #26031

3. because fe could not compile, FE UT CI do not work well. So, some UT failed be introduced by the PRs merged after PR #25933 merged. So this PR revert them to fix FE UT

> - Revert "[Bug](materialized-view) SelectMaterializedIndexWithAggregate do not change plan > when match ba… (#26145)"
> This reverts commit 8d7abf6.

> - Revert "[enhancement](Nereids): optimize GroupExpressionMatching (#26130)"
> This reverts commit 19122b5.

> - Revert "[Performance](Nereids): optimize GroupExpressionMatching (#26084)"
> This reverts commit 0d956e9.
  • Loading branch information
@morrySnow
morrySnow authored Nov 1, 2023
1 parent d3c475b commit 18dabe7
Show file tree
Hide file tree
Showing 27 changed files with 201 additions and 261 deletions.
3 changes: 0 additions & 3 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/CreateMaterializedViewStmt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,6 @@ private void analyzeFromClause() throws AnalysisException {
if (tableRefList.size() != 1) {
throw new AnalysisException("The materialized view only support one table in from clause.");
}
if (!isReplay && tableRefList.get(0).hasExplicitAlias()) {
throw new AnalysisException("The materialized view not support table with alias.");
}
TableName tableName = tableRefList.get(0).getName();
if (tableName == null) {
throw new AnalysisException("table in from clause is invalid, please check if it's single table "
Expand Down
4 changes: 2 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -723,12 +723,12 @@ public String toSql(boolean isUniqueTable, boolean isCompatible) {
// show change datetimeV2/dateV2 to datetime/date
if (isCompatible) {
if (type.isDatetimeV2()) {
sb.append("datetime");
sb.append("DATETIME");
if (((ScalarType) type).getScalarScale() > 0) {
sb.append("(").append(((ScalarType) type).getScalarScale()).append(")");
}
} else if (type.isDateV2()) {
sb.append("date");
sb.append("DATE");
} else if (type.isDecimalV3()) {
sb.append("DECIMAL");
ScalarType sType = (ScalarType) type;
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public ApplyRuleJob(GroupExpression groupExpression, Rule rule, JobContext conte
}

@Override
public final void execute() throws AnalysisException {
public void execute() throws AnalysisException {
if (groupExpression.hasApplied(rule)
|| groupExpression.isUnused()) {
return;
Expand Down
102 changes: 57 additions & 45 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.doris.nereids.memo.Group;
import org.apache.doris.nereids.memo.GroupExpression;
import org.apache.doris.nereids.properties.LogicalProperties;
import org.apache.doris.nereids.trees.plans.GroupPlan;
import org.apache.doris.nereids.trees.plans.Plan;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -54,7 +55,6 @@ public GroupExpressionIterator iterator() {
public static class GroupExpressionIterator implements Iterator<Plan> {
private final List<Plan> results = Lists.newArrayList();
private int resultIndex = 0;
private int resultsSize;

/**
* Constructor.
Expand All @@ -69,19 +69,21 @@ public GroupExpressionIterator(Pattern<Plan> pattern, GroupExpression groupExpre

int childrenGroupArity = groupExpression.arity();
int patternArity = pattern.arity();
// (logicalFilter(), multi()) match (logicalFilter()),
// but (logicalFilter(), logicalFilter(), multi()) not match (logicalFilter())
boolean extraMulti = patternArity == childrenGroupArity + 1
&& (pattern.hasMultiChild() || pattern.hasMultiGroupChild());
if (patternArity > childrenGroupArity && !extraMulti) {
return;
}
if (!(pattern instanceof SubTreePattern)) {
// (logicalFilter(), multi()) match (logicalFilter()),
// but (logicalFilter(), logicalFilter(), multi()) not match (logicalFilter())
boolean extraMulti = patternArity == childrenGroupArity + 1
&& (pattern.hasMultiChild() || pattern.hasMultiGroupChild());
if (patternArity > childrenGroupArity && !extraMulti) {
return;
}

// (multi()) match (logicalFilter(), logicalFilter()),
// but (logicalFilter()) not match (logicalFilter(), logicalFilter())
if (!pattern.isAny() && patternArity < childrenGroupArity
&& !pattern.hasMultiChild() && !pattern.hasMultiGroupChild()) {
return;
// (multi()) match (logicalFilter(), logicalFilter()),
// but (logicalFilter()) not match (logicalFilter(), logicalFilter())
if (!pattern.isAny() && patternArity < childrenGroupArity
&& !pattern.hasMultiChild() && !pattern.hasMultiGroupChild()) {
return;
}
}

// Pattern.GROUP / Pattern.MULTI / Pattern.MULTI_GROUP can not match GroupExpression
Expand All @@ -91,7 +93,7 @@ public GroupExpressionIterator(Pattern<Plan> pattern, GroupExpression groupExpre

// getPlan return the plan with GroupPlan as children
Plan root = groupExpression.getPlan();
if (patternArity == 0) {
if (patternArity == 0 && !(pattern instanceof SubTreePattern)) {
if (pattern.matchPredicates(root)) {
// if no children pattern, we treat all children as GROUP. e.g. Pattern.ANY.
// leaf plan will enter this branch too, e.g. logicalRelation().
Expand All @@ -101,16 +103,20 @@ public GroupExpressionIterator(Pattern<Plan> pattern, GroupExpression groupExpre
// matching children group, one List<Plan> per child
// first dimension is every child group's plan
// second dimension is all matched plan in one group
List<Plan>[] childrenPlans = new List[childrenGroupArity];
List<List<Plan>> childrenPlans = Lists.newArrayListWithCapacity(childrenGroupArity);
for (int i = 0; i < childrenGroupArity; ++i) {
Group childGroup = groupExpression.child(i);
List<Plan> childrenPlan = matchingChildGroup(pattern, childGroup, i);

if (childrenPlan.isEmpty()) {
// current pattern is match but children patterns not match
return;
if (pattern instanceof SubTreePattern) {
childrenPlan = ImmutableList.of(new GroupPlan(childGroup));
} else {
// current pattern is match but children patterns not match
return;
}
}
childrenPlans[i] = childrenPlan;
childrenPlans.add(childrenPlan);
}
assembleAllCombinationPlanTree(root, pattern, groupExpression, childrenPlans);
} else if (patternArity == 1 && (pattern.hasMultiChild() || pattern.hasMultiGroupChild())) {
Expand All @@ -121,22 +127,25 @@ public GroupExpressionIterator(Pattern<Plan> pattern, GroupExpression groupExpre
results.add(root);
}
}
this.resultsSize = results.size();
}

private List<Plan> matchingChildGroup(Pattern<? extends Plan> parentPattern,
Group childGroup, int childIndex) {
Pattern<? extends Plan> childPattern;
boolean isLastPattern = childIndex + 1 >= parentPattern.arity();
int patternChildIndex = isLastPattern ? parentPattern.arity() - 1 : childIndex;

childPattern = parentPattern.child(patternChildIndex);
// translate MULTI and MULTI_GROUP to ANY and GROUP
if (isLastPattern) {
if (childPattern.isMulti()) {
childPattern = Pattern.ANY;
} else if (childPattern.isMultiGroup()) {
childPattern = Pattern.GROUP;
if (parentPattern instanceof SubTreePattern) {
childPattern = parentPattern;
} else {
boolean isLastPattern = childIndex + 1 >= parentPattern.arity();
int patternChildIndex = isLastPattern ? parentPattern.arity() - 1 : childIndex;

childPattern = parentPattern.child(patternChildIndex);
// translate MULTI and MULTI_GROUP to ANY and GROUP
if (isLastPattern) {
if (childPattern.isMulti()) {
childPattern = Pattern.ANY;
} else if (childPattern.isMultiGroup()) {
childPattern = Pattern.GROUP;
}
}
}

Expand All @@ -145,45 +154,48 @@ private List<Plan> matchingChildGroup(Pattern<? extends Plan> parentPattern,
}

private void assembleAllCombinationPlanTree(Plan root, Pattern<Plan> rootPattern,
GroupExpression groupExpression, List<Plan>[] childrenPlans) {
int childrenPlansSize = childrenPlans.length;
int[] childrenPlanIndex = new int[childrenPlansSize];
GroupExpression groupExpression,
List<List<Plan>> childrenPlans) {
int[] childrenPlanIndex = new int[childrenPlans.size()];
int offset = 0;
LogicalProperties logicalProperties = groupExpression.getOwnerGroup().getLogicalProperties();

// assemble all combination of plan tree by current root plan and children plan
Optional<GroupExpression> groupExprOption = Optional.of(groupExpression);
Optional<LogicalProperties> logicalPropOption = Optional.of(logicalProperties);
while (offset < childrenPlansSize) {
ImmutableList.Builder<Plan> childrenBuilder = ImmutableList.builderWithExpectedSize(childrenPlansSize);
for (int i = 0; i < childrenPlansSize; i++) {
childrenBuilder.add(childrenPlans[i].get(childrenPlanIndex[i]));
while (offset < childrenPlans.size()) {
ImmutableList.Builder<Plan> childrenBuilder =
ImmutableList.builderWithExpectedSize(childrenPlans.size());
for (int i = 0; i < childrenPlans.size(); i++) {
childrenBuilder.add(childrenPlans.get(i).get(childrenPlanIndex[i]));
}
List<Plan> children = childrenBuilder.build();

// assemble children: replace GroupPlan to real plan,
// withChildren will erase groupExpression, so we must
// withGroupExpression too.
Plan rootWithChildren = root.withGroupExprLogicalPropChildren(groupExprOption,
logicalPropOption, children);
Plan rootWithChildren = root.withGroupExprLogicalPropChildren(Optional.of(groupExpression),
Optional.of(logicalProperties), children);
if (rootPattern.matchPredicates(rootWithChildren)) {
results.add(rootWithChildren);
}
for (offset = 0; offset < childrenPlansSize; offset++) {
offset = 0;
while (true) {
childrenPlanIndex[offset]++;
if (childrenPlanIndex[offset] == childrenPlans[offset].size()) {
// Reset the index when it reaches the size of the current child plan list
if (childrenPlanIndex[offset] == childrenPlans.get(offset).size()) {
childrenPlanIndex[offset] = 0;
offset++;
if (offset == childrenPlans.size()) {
break;
}
} else {
break; // Break the loop when the index is within the size of the current child plan list
break;
}
}
}
}

@Override
public boolean hasNext() {
return resultIndex < resultsSize;
return resultIndex < results.size();
}

@Override
Expand Down
5 changes: 5 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public static List<Plan> getAllMatchingPlans(Pattern pattern, Group group) {
matchingPlans.add(plan);
}
}
for (GroupExpression groupExpression : group.getPhysicalExpressions()) {
for (Plan plan : new GroupExpressionMatching(pattern, groupExpression)) {
matchingPlans.add(plan);
}
}
}
return matchingPlans;
}
Expand Down
2 changes: 2 additions & 0 deletions ...ore/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@
public class EnforceMissingPropertiesHelper {
private static final EventProducer ENFORCER_TRACER = new EventProducer(EnforcerEvent.class,
EventChannel.getDefaultChannel().addConsumers(new LogConsumer(EnforcerEvent.class, EventChannel.LOG)));
private final JobContext context;
private final GroupExpression groupExpression;
private Cost curTotalCost;

public EnforceMissingPropertiesHelper(JobContext context, GroupExpression groupExpression,
Cost curTotalCost) {
this.context = context;
this.groupExpression = groupExpression;
this.curTotalCost = curTotalCost;
}
Expand Down
40 changes: 0 additions & 40 deletions .../java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ public List<Rule> buildRules() {
agg.getGroupByExpressions(),
new HashSet<>(agg.getExpressions()));

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);

Expand Down Expand Up @@ -166,10 +162,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -215,10 +207,6 @@ public List<Rule> buildRules() {
collectRequireExprWithAggAndProject(agg.getExpressions(), project.getProjects())
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -277,10 +265,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -338,10 +322,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -389,10 +369,6 @@ public List<Rule> buildRules() {
nonVirtualGroupByExprs(agg),
new HashSet<>(agg.getExpressions()));

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan = scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);

Expand Down Expand Up @@ -446,10 +422,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -502,10 +474,6 @@ public List<Rule> buildRules() {
collectRequireExprWithAggAndProject(agg.getExpressions(), project.getProjects())
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -571,10 +539,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down Expand Up @@ -641,10 +605,6 @@ public List<Rule> buildRules() {
requiredExpr
);

if (result.indexId == scan.getTable().getBaseIndexId()) {
return ctx.root;
}

LogicalOlapScan mvPlan =
scan.withMaterializedIndexSelected(result.preAggStatus, result.indexId);
SlotContext slotContext = generateBaseScanExprToMvExpr(mvPlan);
Expand Down
13 changes: 13 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/trees/AbstractTreeNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

package org.apache.doris.nereids.trees;

import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator;
import org.apache.doris.nereids.trees.plans.ObjectId;
import org.apache.doris.planner.PlanNodeId;

import com.google.common.collect.ImmutableList;

import java.util.List;
Expand All @@ -29,6 +33,7 @@
*/
public abstract class AbstractTreeNode<NODE_TYPE extends TreeNode<NODE_TYPE>>
implements TreeNode<NODE_TYPE> {
protected final ObjectId id = StatementScopeIdGenerator.newObjectId();
protected final List<NODE_TYPE> children;
// TODO: Maybe we should use a GroupPlan to avoid TreeNode hold the GroupExpression.
// https://github.com/apache/doris/pull/9807#discussion_r884829067
Expand All @@ -54,4 +59,12 @@ public List<NODE_TYPE> children() {
public int arity() {
return children.size();
}

/**
* used for PhysicalPlanTranslator only
* @return PlanNodeId
*/
public PlanNodeId translatePlanNodeId() {
return id.toPlanNodeId();
}
}
Loading

0 comments on commit 18dabe7

Please sign in to comment.