You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/06/16 14:12:47 UTC
[doris] branch master updated: [opt](Nereids) add more unexpected expression check (#20901)
This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 7ee744ff5a [opt](Nereids) add more unexpected expression check (#20901)
7ee744ff5a is described below
commit 7ee744ff5a3d2242d18533b2129284c2476545e3
Author: morrySnow <10...@users.noreply.github.com>
AuthorDate: Fri Jun 16 22:12:39 2023 +0800
[opt](Nereids) add more unexpected expression check (#20901)
1. check sub-query after rewrite, should not meet any sub-query there
2. check below expression on specific plan
- Aggreagate
- TableGeneratingFunction
- Filter
- AggregateFunction
- GroupingScalarFunction
- TableGeneratingFunction
- WindowExpression
- Generate
- AggregateFunction
- GroupingScalarFunction
- WindowExpression
- Having
- TableGeneratingFunction
- WindowExpression
- Join
- AggregateFunction
- GroupingScalarFunction
- TableGeneratingFunction
- WindowExpression
- Project
- TableGeneratingFunction
- Sort
- AggregateFunction
- GroupingScalarFunction
- TableGeneratingFunction
- WindowExpression
- Window
- GroupingScalarFunction
- TableGeneratingFunction
---
.../doris/nereids/jobs/executor/Rewriter.java | 2 +-
.../org/apache/doris/nereids/rules/RuleType.java | 2 +-
.../nereids/rules/analysis/CheckAfterRewrite.java | 32 +++++++++++++++++
.../nereids/rules/analysis/CheckAnalysis.java | 40 +++++++++++++++++++---
.../apache/doris/nereids/trees/plans/PlanType.java | 2 ++
.../trees/plans/logical/LogicalGenerate.java | 2 +-
.../trees/plans/physical/PhysicalGenerate.java | 2 +-
7 files changed, 73 insertions(+), 9 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java
index 73ada5fca1..9697a8d007 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java
@@ -213,7 +213,7 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(new PushFilterInsideJoin())
),
- custom(RuleType.CHECK_DATATYPES, CheckDataTypes::new),
+ custom(RuleType.CHECK_DATA_TYPES, CheckDataTypes::new),
// this rule should invoke after ColumnPruning
custom(RuleType.ELIMINATE_UNNECESSARY_PROJECT, EliminateUnnecessaryProject::new),
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
index f2b7fcb177..79d89a35f8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
@@ -89,7 +89,7 @@ public enum RuleType {
CHECK_ANALYSIS(RuleTypeClass.CHECK),
CHECK_OBJECT_TYPE_ANALYSIS(RuleTypeClass.CHECK),
CHECK_BOUND(RuleTypeClass.CHECK),
- CHECK_DATATYPES(RuleTypeClass.CHECK),
+ CHECK_DATA_TYPES(RuleTypeClass.CHECK),
// rewrite rules
NORMALIZE_AGGREGATE(RuleTypeClass.REWRITE),
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAfterRewrite.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAfterRewrite.java
index 554342d406..7e7c1e55dd 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAfterRewrite.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAfterRewrite.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.rules.analysis;
+import org.apache.doris.catalog.AggregateFunction;
import org.apache.doris.catalog.Type;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
@@ -28,10 +29,14 @@ import org.apache.doris.nereids.trees.expressions.Match;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotNotFromChildren;
+import org.apache.doris.nereids.trees.expressions.SubqueryExpr;
import org.apache.doris.nereids.trees.expressions.VirtualSlotReference;
import org.apache.doris.nereids.trees.expressions.WindowExpression;
import org.apache.doris.nereids.trees.expressions.functions.ExpressionTrait;
+import org.apache.doris.nereids.trees.expressions.functions.generator.TableGeneratingFunction;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.GroupingScalarFunction;
import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.algebra.Generate;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
@@ -54,12 +59,39 @@ public class CheckAfterRewrite extends OneAnalysisRuleFactory {
public Rule build() {
return any().then(plan -> {
checkAllSlotReferenceFromChildren(plan);
+ checkUnexpectedExpression(plan);
checkMetricTypeIsUsedCorrectly(plan);
checkMatchIsUsedCorrectly(plan);
return null;
}).toRule(RuleType.CHECK_ANALYSIS);
}
+ private void checkUnexpectedExpression(Plan plan) {
+ if (plan.getExpressions().stream().anyMatch(e -> e.anyMatch(SubqueryExpr.class::isInstance))) {
+ throw new AnalysisException("Subquery is not allowed in " + plan.getType());
+ }
+ if (!(plan instanceof Generate)) {
+ if (plan.getExpressions().stream().anyMatch(e -> e.anyMatch(TableGeneratingFunction.class::isInstance))) {
+ throw new AnalysisException("table generating function is not allowed in " + plan.getType());
+ }
+ }
+ if (!(plan instanceof LogicalAggregate || plan instanceof LogicalWindow)) {
+ if (plan.getExpressions().stream().anyMatch(e -> e.anyMatch(AggregateFunction.class::isInstance))) {
+ throw new AnalysisException("aggregate function is not allowed in " + plan.getType());
+ }
+ }
+ if (!(plan instanceof LogicalAggregate)) {
+ if (plan.getExpressions().stream().anyMatch(e -> e.anyMatch(GroupingScalarFunction.class::isInstance))) {
+ throw new AnalysisException("grouping scalar function is not allowed in " + plan.getType());
+ }
+ }
+ if (!(plan instanceof LogicalWindow)) {
+ if (plan.getExpressions().stream().anyMatch(e -> e.anyMatch(WindowExpression.class::isInstance))) {
+ throw new AnalysisException("analytic function is not allowed in " + plan.getType());
+ }
+ }
+ }
+
private void checkAllSlotReferenceFromChildren(Plan plan) {
Set<Slot> notFromChildren = plan.getExpressions().stream()
.flatMap(expr -> expr.getInputSlots().stream())
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
index c37c05084f..bbe2a52469 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
@@ -21,16 +21,21 @@ import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Expression;
-import org.apache.doris.nereids.trees.expressions.SubqueryExpr;
import org.apache.doris.nereids.trees.expressions.WindowExpression;
import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
+import org.apache.doris.nereids.trees.expressions.functions.generator.TableGeneratingFunction;
import org.apache.doris.nereids.trees.expressions.functions.scalar.GroupingScalarFunction;
import org.apache.doris.nereids.trees.expressions.typecoercion.TypeCheckResult;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
+import org.apache.doris.nereids.trees.plans.logical.LogicalGenerate;
+import org.apache.doris.nereids.trees.plans.logical.LogicalHaving;
import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
+import org.apache.doris.nereids.trees.plans.logical.LogicalWindow;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -50,11 +55,36 @@ public class CheckAnalysis implements AnalysisRuleFactory {
private static final Map<Class<? extends LogicalPlan>, Set<Class<? extends Expression>>>
UNEXPECTED_EXPRESSION_TYPE_MAP = ImmutableMap.<Class<? extends LogicalPlan>,
Set<Class<? extends Expression>>>builder()
+ .put(LogicalAggregate.class, ImmutableSet.of(
+ TableGeneratingFunction.class))
.put(LogicalFilter.class, ImmutableSet.of(
- AggregateFunction.class,
- GroupingScalarFunction.class,
- WindowExpression.class))
- .put(LogicalJoin.class, ImmutableSet.of(SubqueryExpr.class))
+ AggregateFunction.class,
+ GroupingScalarFunction.class,
+ TableGeneratingFunction.class,
+ WindowExpression.class))
+ .put(LogicalGenerate.class, ImmutableSet.of(
+ AggregateFunction.class,
+ GroupingScalarFunction.class,
+ WindowExpression.class))
+ .put(LogicalHaving.class, ImmutableSet.of(
+ TableGeneratingFunction.class,
+ WindowExpression.class))
+ .put(LogicalJoin.class, ImmutableSet.of(
+ AggregateFunction.class,
+ GroupingScalarFunction.class,
+ TableGeneratingFunction.class,
+ WindowExpression.class))
+ .put(LogicalProject.class, ImmutableSet.of(
+ TableGeneratingFunction.class))
+ .put(LogicalSort.class, ImmutableSet.of(
+ AggregateFunction.class,
+ GroupingScalarFunction.class,
+ TableGeneratingFunction.class,
+ WindowExpression.class))
+ .put(LogicalWindow.class, ImmutableSet.of(
+ GroupingScalarFunction.class,
+ TableGeneratingFunction.class
+ ))
.build();
@Override
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java
index 6e1087a6e3..b5e6219d7f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/PlanType.java
@@ -38,6 +38,7 @@ public enum PlanType {
LOGICAL_TVF_RELATION,
LOGICAL_PROJECT,
LOGICAL_FILTER,
+ LOGICAL_GENERATE,
LOGICAL_JOIN,
LOGICAL_AGGREGATE,
LOGICAL_REPEAT,
@@ -83,6 +84,7 @@ public enum PlanType {
PHYSICAL_SCHEMA_SCAN,
PHYSICAL_PROJECT,
PHYSICAL_FILTER,
+ PHYSICAL_GENERATE,
PHYSICAL_BROADCAST_HASH_JOIN,
PHYSICAL_AGGREGATE,
PHYSICAL_REPEAT,
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java
index 5832a45326..12d0b12539 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalGenerate.java
@@ -50,7 +50,7 @@ public class LogicalGenerate<CHILD_TYPE extends Plan> extends LogicalUnary<CHILD
public LogicalGenerate(List<Function> generators, List<Slot> generatorOutput,
Optional<GroupExpression> groupExpression,
Optional<LogicalProperties> logicalProperties, CHILD_TYPE child) {
- super(PlanType.LOGICAL_FILTER, groupExpression, logicalProperties, child);
+ super(PlanType.LOGICAL_GENERATE, groupExpression, logicalProperties, child);
this.generators = ImmutableList.copyOf(generators);
this.generatorOutput = ImmutableList.copyOf(generatorOutput);
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java
index a213c28437..862deb96d2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java
@@ -55,7 +55,7 @@ public class PhysicalGenerate<CHILD_TYPE extends Plan> extends PhysicalUnary<CHI
*/
public PhysicalGenerate(List<Function> generators, List<Slot> generatorOutput,
Optional<GroupExpression> groupExpression, LogicalProperties logicalProperties, CHILD_TYPE child) {
- super(PlanType.PHYSICAL_FILTER, groupExpression, logicalProperties, child);
+ super(PlanType.PHYSICAL_GENERATE, groupExpression, logicalProperties, child);
this.generators = ImmutableList.copyOf(Objects.requireNonNull(generators, "predicates can not be null"));
this.generatorOutput = ImmutableList.copyOf(Objects.requireNonNull(generatorOutput,
"generatorOutput can not be null"));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org