You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by ji...@apache.org on 2023/06/16 12:18:35 UTC
[doris] branch master updated: [fix](match) fix regression case test_index_match_select and test_index_match_phrase (#20860)
This is an automated email from the ASF dual-hosted git repository.
jianliangqi 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 1cc611a913 [fix](match) fix regression case test_index_match_select and test_index_match_phrase (#20860)
1cc611a913 is described below
commit 1cc611a91383b449b63be5bf1323892755d2b11c
Author: YueW <45...@users.noreply.github.com>
AuthorDate: Fri Jun 16 20:18:29 2023 +0800
[fix](match) fix regression case test_index_match_select and test_index_match_phrase (#20860)
1. add more checks for match expression in nereids:
- match expression only support in filter
- match expression left child and right child must all be string type
- left child for match expression must be sloftRef, right child for match expression must be Literal
2. to fix regression case test_index_match_select and test_index_match_phrase
---
.../main/java/org/apache/doris/catalog/Type.java | 2 +-
.../org/apache/doris/analysis/MatchPredicate.java | 9 +++-
.../glue/translator/ExpressionTranslator.java | 35 ++++++++++--
.../doris/nereids/jobs/executor/Rewriter.java | 6 +++
.../org/apache/doris/nereids/rules/RuleType.java | 1 +
.../nereids/rules/analysis/CheckAfterRewrite.java | 17 ++++++
.../rules/expression/rules/FunctionBinder.java | 22 ++++++++
.../rules/rewrite/CheckMatchExpression.java | 62 ++++++++++++++++++++++
.../doris/nereids/trees/expressions/Match.java | 26 ++++-----
9 files changed, 162 insertions(+), 18 deletions(-)
diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java
index 55f386d428..db3bb398ab 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java
@@ -420,7 +420,7 @@ public abstract class Type {
public boolean isStringType() {
return isScalarType(PrimitiveType.VARCHAR)
|| isScalarType(PrimitiveType.CHAR)
- || isScalarType(PrimitiveType.STRING) || isScalarType(PrimitiveType.AGG_STATE);
+ || isScalarType(PrimitiveType.STRING);
}
public boolean isVarcharOrStringType() {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/MatchPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/MatchPredicate.java
index da6a5999d4..8311a183e2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/MatchPredicate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/MatchPredicate.java
@@ -184,8 +184,15 @@ public class MatchPredicate extends Predicate {
/**
* use for Nereids ONLY
*/
- public MatchPredicate(Operator op, Expr e1, Expr e2, Type retType, NullableMode nullableMode) {
+ public MatchPredicate(Operator op, Expr e1, Expr e2, Type retType,
+ NullableMode nullableMode, String invertedIndexParser, String invertedIndexParserMode) {
this(op, e1, e2);
+ if (invertedIndexParser != null) {
+ this.invertedIndexParser = invertedIndexParser;
+ }
+ if (invertedIndexParserMode != null) {
+ this.invertedIndexParserMode = invertedIndexParserMode;
+ }
fn = new Function(new FunctionName(op.name), Lists.newArrayList(e1.getType(), e2.getType()), retType,
false, true, nullableMode);
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
index 211651e31e..d085baf02f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
@@ -30,13 +30,18 @@ import org.apache.doris.analysis.Expr;
import org.apache.doris.analysis.FunctionCallExpr;
import org.apache.doris.analysis.FunctionName;
import org.apache.doris.analysis.FunctionParams;
+import org.apache.doris.analysis.IndexDef;
import org.apache.doris.analysis.IsNullPredicate;
import org.apache.doris.analysis.MatchPredicate;
import org.apache.doris.analysis.OrderByElement;
+import org.apache.doris.analysis.SlotDescriptor;
import org.apache.doris.analysis.SlotRef;
import org.apache.doris.analysis.TimestampArithmeticExpr;
+import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.catalog.Function;
import org.apache.doris.catalog.Function.NullableMode;
+import org.apache.doris.catalog.Index;
+import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.Type;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.AggregateExpression;
@@ -165,12 +170,36 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
@Override
public Expr visitMatch(Match match, PlanTranslatorContext context) {
+ String invertedIndexParser = null;
+ String invertedIndexParserMode = null;
+ SlotRef left = (SlotRef) match.left().accept(this, context);
+ SlotDescriptor slotDesc = left.getDesc();
+ if (slotDesc != null && slotDesc.isScanSlot()) {
+ TupleDescriptor slotParent = slotDesc.getParent();
+ OlapTable olapTbl = (OlapTable) slotParent.getTable();
+ if (olapTbl == null) {
+ throw new AnalysisException("slotRef in matchExpression failed to get OlapTable");
+ }
+ List<Index> indexes = olapTbl.getIndexes();
+ for (Index index : indexes) {
+ if (index.getIndexType() == IndexDef.IndexType.INVERTED) {
+ List<String> columns = index.getColumns();
+ if (left.getColumnName().equals(columns.get(0))) {
+ invertedIndexParser = index.getInvertedIndexParser();
+ invertedIndexParserMode = index.getInvertedIndexParserMode();
+ break;
+ }
+ }
+ }
+ }
MatchPredicate.Operator op = match.op();
return new MatchPredicate(op,
- match.child(0).accept(this, context),
- match.child(1).accept(this, context),
+ match.left().accept(this, context),
+ match.right().accept(this, context),
match.getDataType().toCatalogDataType(),
- NullableMode.DEPEND_ON_ARGUMENT);
+ NullableMode.DEPEND_ON_ARGUMENT,
+ invertedIndexParser,
+ invertedIndexParserMode);
}
@Override
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 43c0ae97f9..73ada5fca1 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
@@ -38,6 +38,7 @@ import org.apache.doris.nereids.rules.rewrite.BuildCTEAnchorAndCTEProducer;
import org.apache.doris.nereids.rules.rewrite.CTEProducerRewrite;
import org.apache.doris.nereids.rules.rewrite.CheckAndStandardizeWindowFunctionAndFrame;
import org.apache.doris.nereids.rules.rewrite.CheckDataTypes;
+import org.apache.doris.nereids.rules.rewrite.CheckMatchExpression;
import org.apache.doris.nereids.rules.rewrite.CollectFilterAboveConsumer;
import org.apache.doris.nereids.rules.rewrite.CollectProjectAboveConsumer;
import org.apache.doris.nereids.rules.rewrite.ColumnPruning;
@@ -263,6 +264,11 @@ public class Rewriter extends AbstractBatchJobExecutor {
bottomUp(RuleSet.PUSH_DOWN_FILTERS),
custom(RuleType.ELIMINATE_UNNECESSARY_PROJECT, EliminateUnnecessaryProject::new)
),
+ topic("Match expression check",
+ topDown(
+ new CheckMatchExpression()
+ )
+ ),
// this rule batch must keep at the end of rewrite to do some plan check
topic("Final rewrite and check",
custom(RuleType.ENSURE_PROJECT_ON_TOP_JOIN, EnsureProjectOnTopJoin::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 7dd712d1e5..f2b7fcb177 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
@@ -97,6 +97,7 @@ public enum RuleType {
NORMALIZE_REPEAT(RuleTypeClass.REWRITE),
EXTRACT_AND_NORMALIZE_WINDOW_EXPRESSIONS(RuleTypeClass.REWRITE),
CHECK_AND_STANDARDIZE_WINDOW_FUNCTION_AND_FRAME(RuleTypeClass.REWRITE),
+ CHECK_MATCH_EXPRESSION(RuleTypeClass.REWRITE),
AGGREGATE_DISASSEMBLE(RuleTypeClass.REWRITE),
SIMPLIFY_AGG_GROUP_BY(RuleTypeClass.REWRITE),
DISTINCT_AGGREGATE_DISASSEMBLE(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 961417691a..554342d406 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
@@ -24,6 +24,7 @@ import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
+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;
@@ -32,6 +33,8 @@ import org.apache.doris.nereids.trees.expressions.WindowExpression;
import org.apache.doris.nereids.trees.expressions.functions.ExpressionTrait;
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.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.trees.plans.logical.LogicalWindow;
@@ -52,6 +55,7 @@ public class CheckAfterRewrite extends OneAnalysisRuleFactory {
return any().then(plan -> {
checkAllSlotReferenceFromChildren(plan);
checkMetricTypeIsUsedCorrectly(plan);
+ checkMatchIsUsedCorrectly(plan);
return null;
}).toRule(RuleType.CHECK_ANALYSIS);
}
@@ -131,4 +135,17 @@ public class CheckAfterRewrite extends OneAnalysisRuleFactory {
});
}
}
+
+ private void checkMatchIsUsedCorrectly(Plan plan) {
+ if (plan.getExpressions().stream().anyMatch(
+ expression -> expression instanceof Match)) {
+ if (plan instanceof LogicalFilter && plan.child(0) instanceof LogicalOlapScan) {
+ return;
+ } else {
+ throw new AnalysisException(String.format(
+ "Not support match in %s in plan: %s, only support in olapScan filter",
+ plan.child(0), plan));
+ }
+ }
+ }
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FunctionBinder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FunctionBinder.java
index c2c48de051..2ab66cc1b6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FunctionBinder.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FunctionBinder.java
@@ -39,6 +39,7 @@ import org.apache.doris.nereids.trees.expressions.InPredicate;
import org.apache.doris.nereids.trees.expressions.InSubquery;
import org.apache.doris.nereids.trees.expressions.IntegralDivide;
import org.apache.doris.nereids.trees.expressions.ListQuery;
+import org.apache.doris.nereids.trees.expressions.Match;
import org.apache.doris.nereids.trees.expressions.Not;
import org.apache.doris.nereids.trees.expressions.TimestampArithmetic;
import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
@@ -241,4 +242,25 @@ public class FunctionBinder extends AbstractExpressionRewriteRule {
inSubquery.getCorrelateSlots(), ((ListQuery) afterTypeCoercion.right()).getTypeCoercionExpr(),
inSubquery.isNot());
}
+
+ @Override
+ public Expression visitMatch(Match match, ExpressionRewriteContext context) {
+ Expression left = match.left().accept(this, context);
+ Expression right = match.right().accept(this, context);
+ // check child type
+ if (!left.getDataType().isStringLikeType()) {
+ throw new AnalysisException(String.format(
+ "left operand '%s' part of predicate " + "'%s' should return type 'STRING' but "
+ + "returns type '%s'.",
+ left.toSql(), match.toSql(), left.getDataType()));
+ }
+
+ if (!right.getDataType().isStringLikeType() && !right.getDataType().isNullType()) {
+ throw new AnalysisException(String.format(
+ "right operand '%s' part of predicate " + "'%s' should return type 'STRING' but "
+ + "returns type '%s'.",
+ right.toSql(), match.toSql(), right.getDataType()));
+ }
+ return match.withChildren(left, right);
+ }
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMatchExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMatchExpression.java
new file mode 100644
index 0000000000..3b04059a48
--- /dev/null
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMatchExpression.java
@@ -0,0 +1,62 @@
+// 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.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+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.Match;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.expressions.literal.Literal;
+import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
+
+import java.util.List;
+
+/**
+ * Check match expression
+ */
+public class CheckMatchExpression extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule build() {
+ return logicalFilter(logicalOlapScan())
+ .when(filter -> containsMatchExpression(filter.getExpressions()))
+ .then(this::checkChildren)
+ .toRule(RuleType.CHECK_MATCH_EXPRESSION);
+ }
+
+ private LogicalFilter checkChildren(LogicalFilter filter) {
+ List<Expression> expressions = filter.getExpressions();
+ for (Expression expr : expressions) {
+ if (expr instanceof Match) {
+ Match matchExpression = (Match) expr;
+ if (!(matchExpression.left() instanceof SlotReference)
+ || !(matchExpression.right() instanceof Literal)) {
+ throw new AnalysisException(String.format(
+ "Only support match left operand is SlotRef, right operand is Literal"));
+ }
+ }
+ }
+ return filter;
+ }
+
+ private boolean containsMatchExpression(List<Expression> expressions) {
+ return expressions.stream().anyMatch(expr -> expr.anyMatch(Match.class::isInstance));
+ }
+}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Match.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Match.java
index ef3b79c196..01241243bb 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Match.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Match.java
@@ -20,22 +20,20 @@ package org.apache.doris.nereids.trees.expressions;
import org.apache.doris.analysis.MatchPredicate.Operator;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.exceptions.UnboundException;
+import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.BooleanType;
import org.apache.doris.nereids.types.DataType;
-
-import com.google.common.base.Preconditions;
+import org.apache.doris.nereids.types.coercion.AbstractDataType;
+import org.apache.doris.nereids.types.coercion.AnyDataType;
/**
* like expression: a MATCH 'hello'.
*/
-public abstract class Match extends Expression {
-
- protected final String symbol;
+public abstract class Match extends BinaryOperator implements PropagateNullable {
public Match(Expression left, Expression right, String symbol) {
- super(left, right);
- this.symbol = symbol;
+ super(left, right, symbol);
}
/**
@@ -60,22 +58,24 @@ public abstract class Match extends Expression {
return BooleanType.INSTANCE;
}
+ @Override
+ public AbstractDataType inputType() {
+ return AnyDataType.INSTANCE;
+ }
+
@Override
public boolean nullable() throws UnboundException {
- Preconditions.checkArgument(children.size() == 2);
- return (children.get(0).nullable() || children.get(1).nullable());
+ return left().nullable() || right().nullable();
}
@Override
public String toSql() {
- Preconditions.checkArgument(children.size() == 2);
- return "(" + children.get(0).toSql() + " " + symbol + " " + children.get(1).toSql() + ")";
+ return "(" + left().toSql() + " " + symbol + " " + right().toSql() + ")";
}
@Override
public String toString() {
- Preconditions.checkArgument(children.size() == 2);
- return "(" + children.get(0).toString() + " " + symbol + " " + children.get(1).toString() + ")";
+ return "(" + left().toString() + " " + symbol + " " + right().toString() + ")";
}
public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org