github-actions[bot] commented on code in PR #69292:
URL: https://github.com/apache/doris/pull/69292#discussion_r4322914767
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckScoreUsage.java:
##########
@@ -17,31 +17,120 @@
package org.apache.doris.nereids.rules.rewrite;
+import org.apache.doris.nereids.StatementContext;
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.Alias;
+import org.apache.doris.nereids.trees.expressions.CTEId;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.WindowExpression;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Score;
+import org.apache.doris.nereids.trees.plans.JoinType;
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.LogicalCTEConsumer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEProducer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCheckPolicy;
+import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
+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.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.trees.plans.logical.LogicalQualify;
+import org.apache.doris.nereids.trees.plans.logical.LogicalRelation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSelectHint;
+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.LogicalUnion;
+import org.apache.doris.nereids.trees.plans.logical.LogicalView;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
/**
- * Check score function usage in project and aggregate without proper
optimization context.
- *
- * This rule ensures that score() function is used only in contexts where it
can be optimized,
- * requiring WHERE clause with MATCH function, ORDER BY and LIMIT.
+ * Reject score() calls outside a supported scan scoring context.
+ * Aggregates cannot consume raw score() calls.
*/
public class CheckScoreUsage implements RewriteRuleFactory {
private static final Logger LOG =
LogManager.getLogger(CheckScoreUsage.class);
+ private static final String SCORE_CONTEXT_ERROR =
+ "score() function requires WHERE clause with MATCH function, ORDER
BY and LIMIT for optimization, "
+ + "or a supported minimum score filter with MATCH or
SEARCH";
+ private static final String SCORE_FILTER_CONTEXT_ERROR =
+ "score() filter must be in the WHERE clause of a query on one
table with a MATCH or SEARCH "
+ + "predicate. It cannot filter the result of an inner,
cross or full join, an aggregation, "
+ + "a window or another operator; put the MATCH or SEARCH
predicate and the score() filter "
+ + "in a subquery on that table";
+
+ /**
+ * Fail a score() filter whose input combines or recomputes rows. score()
has no input slot, so
+ * predicate pushdown would treat it as a constant and move it below a
window, an aggregation
+ * or an inner join, changing the result or filtering another scan. Called
after analysis,
+ * before any pushdown or CTE inlining.
+ */
+ public static void checkScoreFilterContext(Plan plan, StatementContext
statementContext) {
+ if (!(plan instanceof LogicalFilter || plan instanceof LogicalHaving
|| plan instanceof LogicalQualify)) {
+ return;
+ }
+ if (plan.getExpressions().stream().noneMatch(expr ->
expr.containsType(Score.class))) {
+ return;
+ }
+ if (!producesScanRows(plan.child(0), statementContext, new
HashSet<>())) {
+ throw new AnalysisException(SCORE_FILTER_CONTEXT_ERROR);
+ }
+ }
+
+ // Whether every output row is a row of one scan, so that a filter above
can move down to it.
+ // Outer, semi and anti joins keep every row of one side.
+ private static boolean producesScanRows(Plan plan, StatementContext
statementContext,
+ Set<CTEId> visitingCtes) {
+ if (plan instanceof LogicalCTEConsumer) {
+ // The producer decides the rows: a single-use CTE is inlined into
its consumer later.
+ CTEId cteId = ((LogicalCTEConsumer) plan).getCteId();
+ LogicalCTEProducer<? extends Plan> producer =
statementContext.getCteProducerByCteId(cteId);
+ Preconditions.checkState(producer != null, "CTE producer %s is not
analyzed", cteId);
+ // A recursive CTE reads itself and has no single scan behind its
rows.
+ if (!visitingCtes.add(cteId)) {
+ return false;
+ }
+ boolean result = producesScanRows(producer.child(),
statementContext, visitingCtes);
+ visitingCtes.remove(cteId);
+ return result;
+ }
+ if (plan instanceof LogicalRelation) {
+ return true;
+ }
+ if (plan instanceof LogicalJoin) {
+ JoinType joinType = ((LogicalJoin<?, ?>) plan).getJoinType();
+ if (joinType.isLeftJoin()) {
+ return producesScanRows(plan.child(0), statementContext,
visitingCtes);
+ }
+ return joinType.isRightJoin() && producesScanRows(plan.child(1),
statementContext, visitingCtes);
+ }
+ if (plan instanceof LogicalUnion) {
Review Comment:
[P2] Reject raw score filters above UNION DISTINCT
This accepts `LogicalUnion` with the DISTINCT qualifier as a
scan-row-preserving input. A triggering reduced plan is:
```text
Filter(score() > 0.5)
Union DISTINCT
Project(id) -> Filter(c1 MATCH 'apple') -> Scan A
Project(id) -> Filter(c1 MATCH 'apple') -> Scan B
```
The first `PUSH_DOWN_FILTERS` runs before `BuildAggForUnion`, so
`PushDownFilterThroughSetOperation` copies the slot-free comparison into both
branches. The new scan rewrite then binds/removes it before the late score
check, accepting a score filter on deduplicated rows instead of reporting the
unsupported context. Equal projected ids can have different BM25 scores in the
two scans, so the merged output has no single scan-local score; the rewrite
instead retains an id when either source passes. Require `Qualifier.ALL` in
this branch and add a negative UNION DISTINCT test, keeping the supported UNION
ALL behavior.
--
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]
