github-actions[bot] commented on code in PR #69292:
URL: https://github.com/apache/doris/pull/69292#discussion_r4315061210
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -98,7 +76,70 @@ public List<Rule> buildRules() {
LogicalFilter<LogicalOlapScan> filter =
project.child();
LogicalOlapScan scan = filter.child();
return pushDown(topN, project, filter, scan);
-
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN));
+
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN),
+ logicalProject(logicalFilter(logicalOlapScan()))
+ .then(project -> pushDownScoreRange(project))
+ .toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN),
+ logicalFilter(logicalOlapScan())
+ .then(filter -> pushDownScoreRange(filter))
+ .toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN));
+ }
+
+ private LogicalProject<Plan> pushDownScoreRange(
+ LogicalProject<LogicalFilter<LogicalOlapScan>> project) {
+ LogicalFilter<Plan> filter = pushDownScoreRange(project.child());
+ if (filter == null) {
+ return null;
+ }
+ LogicalOlapScan scan = (LogicalOlapScan) filter.child();
+ Expression scoreSlot = scan.getScoreOrderKeys().get(0).getExpr();
+ Map<Expression, Expression> replaceMap = Maps.newHashMap();
+ for (NamedExpression projection : project.getProjects()) {
+ for (Expression score : projection.<Expression>collect(e -> e
instanceof Score)) {
+ replaceMap.put(score, scoreSlot);
+ }
+ }
+ return project.withProjectsAndChild(
+ ExpressionUtils.replaceNamedExpressions(project.getProjects(),
replaceMap), filter);
+ }
+
+ private LogicalFilter<Plan>
pushDownScoreRange(LogicalFilter<LogicalOlapScan> filter) {
+ Optional<Expression> scorePredicateOpt =
findScoreRangePredicate(filter.getConjuncts());
+ if (!scorePredicateOpt.isPresent()) {
+ return null;
+ }
+ boolean hasMatchOrSearch = filter.getConjuncts().stream()
+ .anyMatch(conjunct -> !conjunct.collect(
+ e -> e instanceof Match || e instanceof
SearchExpression).isEmpty());
+ if (!hasMatchOrSearch) {
+ throw new AnalysisException("WHERE clause must contain at least
one MATCH or SEARCH function"
+ + " for score() filtering");
+ }
+ Expression scorePredicate = scorePredicateOpt.get();
+ Optional<ScoreRangeInfo> scoreRangeInfo =
Optional.of(extractScoreRangeInfo(scorePredicate));
+ Set<Expression> scoreFunctions = scorePredicate.collect(e -> e
instanceof Score);
+ Alias scoreAlias = new Alias(scoreFunctions.iterator().next(),
"__score");
+ // A zero score limit enables scoring without discarding candidates
before other filters.
+ LogicalOlapScan scan =
filter.child().appendVirtualColumnsAndTopN(ImmutableList.of(scoreAlias),
+ ImmutableList.of(), Optional.empty(),
+ ImmutableList.of(new OrderKey(scoreAlias.toSlot(), false,
false)), Optional.of(0L),
Review Comment:
[P1] Keep projected MATCH expressions out of the filter score
A reduced triggering plan is:
```text
Project(id, c1 MATCH_PHRASE 'apple' AS flag)
Filter(c1 MATCH 'apple', score() > 0.6)
OlapScan
```
`PushDownMatchProjectionAsVirtualColumn` runs before this rule and has
already appended the projected MATCH to the scan. Enabling scoring here makes
`SegmentIterator::_apply_index_expr` evaluate both the WHERE predicate and that
virtual projection with the same `CollectionSimilarity`; both publish scores
and `collect()` adds them with `+=` before the range is applied. With the
initial three documents in `test_score_filter_matrix`, the WHERE score is
0.4700036, but the projected single-term MATCH adds another 0.4700036. Thus
adding `flag` changes the result from no rows to ids 1 and 2 at threshold 0.6.
Isolate score collection from projection-only MATCH evaluation and cover this
combination with a regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -98,7 +76,70 @@ public List<Rule> buildRules() {
LogicalFilter<LogicalOlapScan> filter =
project.child();
LogicalOlapScan scan = filter.child();
return pushDown(topN, project, filter, scan);
-
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN));
+
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN),
+ logicalProject(logicalFilter(logicalOlapScan()))
+ .then(project -> pushDownScoreRange(project))
+ .toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN),
+ logicalFilter(logicalOlapScan())
+ .then(filter -> pushDownScoreRange(filter))
+ .toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN));
+ }
+
+ private LogicalProject<Plan> pushDownScoreRange(
+ LogicalProject<LogicalFilter<LogicalOlapScan>> project) {
+ LogicalFilter<Plan> filter = pushDownScoreRange(project.child());
+ if (filter == null) {
+ return null;
+ }
+ LogicalOlapScan scan = (LogicalOlapScan) filter.child();
+ Expression scoreSlot = scan.getScoreOrderKeys().get(0).getExpr();
+ Map<Expression, Expression> replaceMap = Maps.newHashMap();
+ for (NamedExpression projection : project.getProjects()) {
+ for (Expression score : projection.<Expression>collect(e -> e
instanceof Score)) {
Review Comment:
[P2] Bind score calls inside existing CSE virtual columns
With `experimental_enable_virtual_slot_for_cse=true`, a supported query such
as `SELECT id, sin(abs(score())) AS a, cos(abs(score())) AS b FROM t WHERE c1
MATCH 'apple' AND score() > 0` reaches this rule as:
```text
Project(id, sin(v), cos(v))
Filter(MATCH, score() > 0)
OlapScan(virtual v = abs(score()))
```
The earlier `PushDownVirtualColumnsIntoOlapScan` extracts the repeated
`abs(score())`. This loop only replaces calls still present in the project, so
it leaves the raw Score inside `v` and appends a separate materialized
`__score`. BE fills only `__score` through `ScoreRuntime`; evaluating `v`
invokes the scalar score placeholder, which always returns zero. Consequently
the projected values are 0 and 1 instead of being derived from BM25. Exclude
score-containing CSE expressions until score binding, or bind their nested
calls to the score slot with correct virtual-column dependency ordering.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -98,7 +76,70 @@ public List<Rule> buildRules() {
LogicalFilter<LogicalOlapScan> filter =
project.child();
LogicalOlapScan scan = filter.child();
return pushDown(topN, project, filter, scan);
-
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN));
+
}).toRule(RuleType.PUSH_DOWN_SCORE_TOPN_INTO_OLAP_SCAN),
+ logicalProject(logicalFilter(logicalOlapScan()))
+ .then(project -> pushDownScoreRange(project))
Review Comment:
[P2] Update the existing regression that expects this query to fail
`regression-test/suites/inverted_index_p0/test_bm25_score.groovy:163-165`
still runs `select *, score() as score from test_bm25_score where request
match_any 'button.03.gif' and score() > 0.5` inside `test { ... exception
"score() function requires ..." }`. This newly registered rewrite now accepts
exactly that Project/Filter/Scan shape, replaces the projected score and
removes the range predicate, so the expected analysis exception disappears.
`TestAction` fails when an expected exception is absent, even if the query
returns no rows; the existing p0 suite therefore breaks. Convert that rejection
case into a positive minimum-score test alongside the new suites.
--
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]
