github-actions[bot] commented on code in PR #65594:
URL: https://github.com/apache/doris/pull/65594#discussion_r3579781546
##########
be/src/exprs/function/function_search.cpp:
##########
@@ -253,13 +265,20 @@ Status
FunctionSearch::evaluate_inverted_index_with_search_param(
const IndexExecContext* index_exec_ctx,
const std::unordered_map<std::string, int>& field_name_to_column_id,
const std::shared_ptr<IndexQueryContext>& index_query_context) const {
- const bool is_nested_query = search_param.root.clause_type == "NESTED";
- if (is_nested_query && !is_nested_group_search_supported()) {
+ const bool has_nested_query =
search_clause_contains_nested(search_param.root);
+ if (has_nested_query && index_query_context &&
index_query_context->collection_similarity) {
+ return Status::InvalidArgument(
+ "NESTED search with score() is not supported until nested
score aggregation is "
+ "implemented");
+ }
+
+ const bool is_top_level_nested_query = search_param.root.clause_type ==
"NESTED";
+ if (is_top_level_nested_query && !is_nested_group_search_supported()) {
return Status::NotSupported(
"NESTED query requires NestedGroup support, which is
unavailable in this build");
}
- if (!is_nested_query && (iterators.empty() ||
data_type_with_names.empty())) {
+ if (!has_nested_query && (iterators.empty() ||
data_type_with_names.empty())) {
Review Comment:
This recursive flag also changes non-score wrapped nested searches on the
no-index path. `best_fields` expansion can serialize a valid top-level
`NESTED(...)` as `OR(NESTED(...))`; before this change that shape still hit the
empty-iterator/data-type fallback because the root was not `NESTED`. Now
`has_nested_query` skips the fallback, but only a top-level `NESTED` is handled
by `VariantNestedSearchEvaluator`, so the same no-index query falls through to
`build_query_recursive()` and returns `INVALID_ARGUMENT` (`NESTED clause must
be evaluated at top level`). That is a behavior regression for plain `search()`
without score. Please keep the existing empty-result fallback for wrapped
nested queries without score, or reject/normalize the wrapped shape before it
reaches BE, and add a regression for the empty-iterator path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckScoreUsage.java:
##########
@@ -66,10 +66,38 @@ public List<Rule> buildRules() {
"score() function cannot be used in aggregate
functions. "
+ "score() requires WHERE clause with MATCH
function, "
+ "ORDER BY and LIMIT for optimization");
+ }).toRule(RuleType.CHECK_SCORE_USAGE),
+
+ logicalFilter()
Review Comment:
The new checks still let `score()` through when it lives in other plan-owned
expression containers. For example, `ExtractAndNormalizeWindowExpression` runs
before this batch and turns `row_number() over(order by score())` into a top
project over `LogicalWindow`; the project only references the window output
slot, while the `Score` remains inside `LogicalWindow.getExpressions()`.
`LogicalJoin.getExpressions()` similarly owns join conjuncts, but this rule
only checks Project/Aggregate/Filter/TopN/Sort, so these unoptimized `score()`
calls can still reach translation as the ordinary builtin that returns zeros
instead of being rejected. Please make the post-pushdown check cover all
plan-owned expressions after exempting the optimized Project/OlapScan shape,
and add negative coverage for cases such as `over(order by score())` and join
predicates if analysis accepts them.
--
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]
