github-actions[bot] commented on code in PR #69292:
URL: https://github.com/apache/doris/pull/69292#discussion_r4322020338


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckScoreUsage.java:
##########
@@ -35,13 +48,67 @@
 import java.util.List;
 
 /**
- * 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.
+     */
+    public static void checkScoreFilterContext(Plan plan) {
+        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))) {
+            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) {
+        if (plan instanceof LogicalRelation) {
+            return true;

Review Comment:
   [P2] Check the producer behind a CTE consumer before accepting its rows
   
   `LogicalCTEConsumer` extends `LogicalRelation`, so this accepts `WITH w AS 
(SELECT id, row_number() OVER (ORDER BY id) AS rn FROM t WHERE c1 MATCH 
'apple') SELECT * FROM w WHERE score() > 0.5` without inspecting the window in 
`w`. Unlike the direct derived-table case in the existing window thread, the 
new early check sees only `Filter(score()>0.5) -> CTEConsumer(w)`.
   
   `CTEInline` expands a single-use CTE after both `CheckAnalysis` passes. 
Window extraction then produces:
   ```text
   Filter(score() > 0.5)
     Window(row_number() OVER (ORDER BY id) AS rn)
       Filter(c1 MATCH 'apple')
         OlapScan
   ```
   `PushDownFilterThroughWindow` accepts the predicate's empty input slots, and 
the standalone score rule binds/removes it below the window before the late 
check runs. With the ANN regression's documents, only matching id 2 exceeds 
0.5, so it receives `rn=1` instead of retaining `rn=2`; this unsupported outer 
score filter should be rejected. Resolve the producer's row semantics, or 
revalidate after inlining and before predicate pushdown, and add a CTE-window 
regression.



-- 
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]

Reply via email to