airborne12 commented on code in PR #69292:
URL: https://github.com/apache/doris/pull/69292#discussion_r4336002920
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java:
##########
@@ -113,22 +113,30 @@ public Plan
visitLogicalCatalogRelation(LogicalCatalogRelation catalogRelation,
newRelation = ((LogicalOlapScan)
newRelation).withPartitionPruned(false);
}
updateReplaceMapWithOutput(catalogRelation, newRelation,
context.exprIdReplaceMap);
- List<NamedExpression> virtualColumns =
catalogRelation.getVirtualColumns().stream()
- .map(e -> {
- if (e instanceof Alias) {
- return new Alias(((Alias) e).child(), e.getName());
- }
- return e;
- })
- .collect(ImmutableList.toImmutableList());
- for (int i = 0; i < virtualColumns.size(); i++) {
-
context.exprIdReplaceMap.put(catalogRelation.getVirtualColumns().get(i).getExprId(),
- virtualColumns.get(i).getExprId());
+ if (!catalogRelation.getVirtualColumns().isEmpty()) {
+ // Each virtual column gets a new id, and its old slot maps to
that id.
+ List<NamedExpression> virtualColumns =
catalogRelation.getVirtualColumns().stream()
+ .map(column -> {
+ NamedExpression newColumn = column instanceof Alias
+ ? new
Alias(ExpressionDeepCopier.INSTANCE.deepCopy(((Alias) column).child(),
+ context), column.getName())
+ : (NamedExpression)
ExpressionDeepCopier.INSTANCE.deepCopy(column, context);
+ context.exprIdReplaceMap.put(column.getExprId(),
newColumn.getExprId());
+ return newColumn;
+ })
+ .collect(ImmutableList.toImmutableList());
+ // The copied relation still outputs the old virtual columns, so
compute its output again.
+ newRelation = (LogicalCatalogRelation)
newRelation.withVirtualColumns(virtualColumns)
+ .withGroupExprLogicalPropChildren(Optional.empty(),
Optional.empty(), ImmutableList.of());
Review Comment:
Confirmed and fixed in cd636a132c. It also reproduces without
`cte_inline_mode=1`: with two consumers and `UNION ALL SELECT * FROM c WHERE 1
= 0`, `Optimizer.normalizeCtePlan` inlines the remaining consumer, and the
recomputed output gave `v['x']` a new id.
The copier no longer computes the output again.
`LogicalOlapScan.withVirtualColumns`, which replaces the virtual columns, now
also replaces their slots in the output and keeps the other slots, including
the subpath slots the copier has already mapped. The same applies to
`LogicalOlapTableStreamScan`. Since the copier bug also exists on master (any
CTE with a score or ANN TopN fails there), the fix is split out to #69399; this
PR carries the identical change until it is merged.
`CTEInlineTest` in #69399 covers your shape (`v['x']` plus a MATCH
projection, one consumer removed) and fails with the previous version of the
fix. `LogicalPlanDeepCopierTest` now also checks that every slot of the
original scan maps to a slot of the copy.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/BindScoreToOlapScan.java:
##########
@@ -0,0 +1,346 @@
+// 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.jobs.JobContext;
+import org.apache.doris.nereids.properties.OrderKey;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Cast;
+import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.GreaterThan;
+import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
+import org.apache.doris.nereids.trees.expressions.LessThan;
+import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+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.functions.scalar.Score;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.Search;
+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.LogicalApply;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEAnchor;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEProducer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
+import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
+import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import
org.apache.doris.nereids.trees.plans.logical.LogicalRecursiveUnionAnchor;
+import
org.apache.doris.nereids.trees.plans.logical.LogicalRecursiveUnionProducer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalRelation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSetOperation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSink;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSubQueryAlias;
+import org.apache.doris.nereids.trees.plans.logical.LogicalView;
+import org.apache.doris.nereids.trees.plans.visitor.CustomRewriter;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+/**
+ * Bind score() to the olap scan that computes it.
+ *
+ * <p>score() is the relevance of a row for the MATCH and SEARCH predicates in
the WHERE clause of
+ * its SELECT, and only the olap scan that evaluates those predicates computes
it. So score() is
+ * supported in a SELECT that reads one olap table and has a MATCH or SEARCH
predicate in its WHERE
+ * clause:
+ * <ul>
+ * <li>in the WHERE clause, as one minimum score: score() > x or score() >=
x;</li>
+ * <li>in the select list, window functions and ORDER BY of a SELECT that
does not aggregate.</li>
+ * </ul>
+ * A SELECT in a subquery, view, CTE or set operation has to ORDER BY score()
with a LIMIT, which keeps
+ * the predicates of the outer query away from its scan. A correlated subquery
cannot call score(), because
+ * decorrelation can remove that LIMIT. Any other use of score() is rejected.
+ *
+ * <p>This rule runs before subquery aliases are removed. It adds a score
column to the scan, which
+ * scores every row the scan returns, and replaces the score() calls of the
SELECT with it.
+ * PushDownScoreRangeIntoOlapScan and PushDownScoreTopNIntoOlapScan push the
minimum score and a
+ * TopN on the score into the scan.
+ */
+public class BindScoreToOlapScan implements CustomRewriter {
+ public static final String UNSUPPORTED_SELECT_ERROR = "score() can only be
used in a SELECT that reads one "
+ + "table and has a MATCH or SEARCH predicate in its WHERE clause";
+ public static final String NESTED_SELECT_ERROR = "score() in a subquery,
view, CTE or set operation "
+ + "requires ORDER BY score() with LIMIT";
+ public static final String CORRELATED_SUBQUERY_ERROR = "score() cannot be
used in a correlated subquery";
+ public static final String AGGREGATE_ERROR = "score() can only be used in
the WHERE clause of a SELECT "
+ + "with GROUP BY, aggregate functions or DISTINCT";
+ public static final String HAVING_OR_QUALIFY_ERROR = "score(), MATCH and
SEARCH cannot be used in HAVING "
+ + "or QUALIFY of a SELECT that calls score()";
+ public static final String ONE_RANGE_ERROR = "Only one score() range
predicate is supported in WHERE clause";
+ public static final String RANGE_FORM_ERROR = "score() predicate in WHERE
clause must be in the form of "
+ + "'score() > literal' or 'score() >= literal'";
+ public static final String TOP_LEVEL_RANGE_ERROR = "score() predicate must
be a top-level AND condition "
+ + "in WHERE clause";
+
+ @Override
+ public Plan rewriteRoot(Plan plan, JobContext jobContext) {
+ if (!plan.anyMatch(node -> callsScore((Plan) node))) {
+ return plan;
+ }
+ return bindSelect(plan, true);
+ }
+
+ /** The score column that this rule added to the scan. */
+ public static Optional<Slot> findScoreSlot(LogicalOlapScan scan) {
+ return scan.getVirtualColumns().stream()
+ .filter(column -> column instanceof Alias && ((Alias)
column).child() instanceof Score)
+ .map(NamedExpression::toSlot)
+ .findFirst();
+ }
+
+ private Plan bindSelect(Plan root, boolean outermost) {
+ Plan plan = bindNestedSelects(root);
+ if (!selectCallsScore(plan)) {
+ return plan;
+ }
+ List<Plan> path = pathToScan(plan);
+ if (path.isEmpty()) {
+ throw new AnalysisException(UNSUPPORTED_SELECT_ERROR);
+ }
+ check(path, outermost);
+ return bind(path);
+ }
+
+ private Plan bindNestedSelects(Plan plan) {
+ ImmutableList.Builder<Plan> newChildren =
ImmutableList.builderWithExpectedSize(plan.arity());
+ boolean changed = false;
+ for (int i = 0; i < plan.arity(); i++) {
+ Plan child = plan.child(i);
+ // Decorrelation can remove the LIMIT and ORDER BY of a SELECT in
a correlated subquery.
+ if (plan instanceof LogicalApply && i == 1 && ((LogicalApply<?,
?>) plan).isCorrelated()
+ && child.anyMatch(node -> callsScore((Plan) node))) {
+ throw new AnalysisException(CORRELATED_SUBQUERY_ERROR);
+ }
+ Plan newChild = startsSelect(plan, i) ? bindSelect(child, false) :
bindNestedSelects(child);
+ changed |= newChild != child;
+ newChildren.add(newChild);
+ }
+ return changed ? plan.withChildren(newChildren.build()) : plan;
+ }
+
+ // Subqueries, views, CTEs and the inputs of set operations are SELECTs of
their own, a table
+ // alias is not.
+ private static boolean startsSelect(Plan parent, int childIndex) {
+ if (parent instanceof LogicalSubQueryAlias) {
+ return !(parent.child(0) instanceof LogicalRelation);
+ }
+ return parent instanceof LogicalView || parent instanceof
LogicalCTEProducer
+ || parent instanceof LogicalSetOperation || parent instanceof
LogicalRecursiveUnionAnchor
+ || parent instanceof LogicalRecursiveUnionProducer
+ || parent instanceof LogicalCTEAnchor && childIndex == 0
+ || parent instanceof LogicalApply && childIndex == 1;
+ }
+
+ private static boolean selectCallsScore(Plan plan) {
+ if (callsScore(plan)) {
+ return true;
+ }
+ for (int i = 0; i < plan.arity(); i++) {
+ if (!startsSelect(plan, i) && selectCallsScore(plan.child(i))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean callsScore(Plan plan) {
+ return plan.getExpressions().stream().anyMatch(expression ->
expression.containsType(Score.class));
+ }
+
+ // The plan nodes from the root of the SELECT down to its olap scan; empty
when the SELECT reads
+ // anything else than one olap table, e.g. a join, a subquery or a lateral
view.
+ private static List<Plan> pathToScan(Plan root) {
+ List<Plan> path = new ArrayList<>();
+ Plan node = root;
+ while (!(node instanceof LogicalOlapScan)) {
+ path.add(node);
+ if (node instanceof LogicalCTEAnchor) {
+ node = node.child(1);
+ } else if (node instanceof LogicalSink || node instanceof
LogicalLimit || node instanceof LogicalSort
+ || node instanceof LogicalProject || node instanceof
LogicalFilter
+ || node instanceof LogicalAggregate
Review Comment:
Fixed in cd636a132c. `pathToScan` now walks through `LogicalRepeat` like the
aggregate above it. So a minimum score works with GROUPING SETS, ROLLUP and
CUBE, and score() in such a SELECT gets the aggregate error.
`BindScoreToOlapScanTest` covers all three forms and `max(score())` with
ROLLUP; `test_score_filter_usage` has your GROUPING SETS query
(`grouping_sets_score_range`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
