github-actions[bot] commented on code in PR #65594:
URL: https://github.com/apache/doris/pull/65594#discussion_r3578269258
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownScoreTopNIntoOlapScan.java:
##########
@@ -128,6 +128,18 @@ private Plan pushDown(
+ " for score() push down optimization");
}
+ // Nested search currently produces only a parent-row bitmap. It does
not collect or
+ // aggregate element scores into CollectionSimilarity, so allowing
score() pushdown would
+ // silently materialize zero scores. Reject this combination until
nested scoring semantics
+ // are implemented.
+ boolean hasTopLevelNestedSearch = filter.getConjuncts().stream()
+ .anyMatch(conjunct -> !conjunct.collect(e -> e instanceof
SearchExpression
Review Comment:
This check is too narrow for the unsupported nested-score combinations. It
only looks at `SearchExpression.isTopLevelNestedQuery()` inside the score TopN
pushdown path after the project already contains `Alias(Score)`, but there are
reachable shapes that avoid that:
1. `SearchDslParser` can produce a tree that still contains `NESTED` below
another root. In standard multi-field `best_fields` mode,
`parseDslMultiFieldMode()` validates the original root while it is `NESTED`,
then `expandBestFields()` wraps field copies in a new `OR`, so
`search('NESTED(data, msg:hello)',
'{"mode":"standard","fields":["data","other"]}')` becomes `OR(NESTED(...),
NESTED(...))`. `isTopLevelNestedQuery()` returns false, and the BE guard added
in this PR also misses it because it checks only `root.clause_type == "NESTED"`.
2. `score()` can remain outside the projected TopN shape. For example,
`SELECT id FROM t WHERE search('NESTED(data, msg:hello)',
'{"mode":"standard"}') AND score() >= 0 ORDER BY id LIMIT 10` returns before
this check because the project has no `Alias(Score)`, and `CheckScoreUsage`
does not inspect filters.
3. The same applies to ORDER BY-only score usage, e.g. `SELECT id FROM t
WHERE search('NESTED(data, msg:hello)', '{"mode":"standard"}') ORDER BY score()
DESC LIMIT 10`. `FillUpMissingSlots` does not synthesize a project alias for
zero-argument `score()`, and `CheckScoreUsage` does not inspect TopN/Sort order
keys.
In the non-projected cases no score runtime is created, so the BE
`collection_similarity` guard is not reached and the ordinary scalar `score()`
implementation can materialize zeroes. Please make nested detection recursive
and add a final score-usage validation that rejects remaining `Score`
expressions in filters/order keys/non-optimized contexts, or normalize those
usages into the optimized alias shape before this rule runs.
##########
be/src/exprs/function/function_search.cpp:
##########
@@ -258,6 +258,11 @@ Status
FunctionSearch::evaluate_inverted_index_with_search_param(
return Status::NotSupported(
"NESTED query requires NestedGroup support, which is
unavailable in this build");
}
+ if (is_nested_query && index_query_context &&
index_query_context->collection_similarity) {
+ return Status::NotSupported(
+ "NESTED search with score() is not supported until nested
score aggregation is "
Review Comment:
Returning `Status::NotSupported` here makes the new safety net easy to
bypass in the normal scan path. `NotSupported` maps to `NOT_IMPLEMENTED_ERROR`,
and `SegmentIterator::_apply_index_expr()` treats that status as downgradable
and just continues for both pushed common expressions and virtual-column
expressions. So even when this guard fires for a root `NESTED` search with
score runtime, the intended user-facing rejection can be swallowed and later
show up as a generic residual `SearchExpr should not be executed without
inverted index` error, or not surface if the segment is already filtered out.
Please return a non-downgraded semantic error such as `InvalidArgument`, or
special-case this nested-score rejection so `_apply_index_expr()` propagates
it. A BE test through the score-runtime/SegmentIterator path would lock this
down.
--
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]
