github-actions[bot] commented on code in PR #65436:
URL: https://github.com/apache/doris/pull/65436#discussion_r3556182489
##########
be/src/exprs/function/function_search.cpp:
##########
@@ -274,8 +274,13 @@ Status
FunctionSearch::evaluate_inverted_index_with_search_param(
OlapReaderStatistics* outer_stats = index_query_context ?
index_query_context->stats : nullptr;
SCOPED_RAW_TIMER(outer_stats ? &outer_stats->inverted_index_query_timer :
&query_timer_dummy);
- // DSL result cache: reuse InvertedIndexQueryCache with SEARCH_DSL_QUERY
type
- auto* dsl_cache = enable_cache ? InvertedIndexQueryCache::instance() :
nullptr;
+ const bool need_similarity_score =
+ index_query_context && index_query_context->collection_similarity;
+
+ // DSL result cache only stores bitmap/null bitmap. It does not store BM25
scores,
+ // so score() queries must execute scorers to populate
CollectionSimilarity.
+ auto* dsl_cache = (enable_cache && !need_similarity_score) ?
InvertedIndexQueryCache::instance()
+ : nullptr;
Review Comment:
This still lets `search(...)` score queries apply the pushed limit before
the rest of the scan predicates. FE pushes `score_sort_limit` for any filter
containing a `SearchExpression`, but here the score path calls
`collect_multi_segment_top_k()` over only the search scorer; that collector
does not see the current `_row_bitmap` or later predicates. `SegmentIterator`
intersects the returned search bitmap with `_row_bitmap` and applies remaining
predicates afterward, so a query like `WHERE search('title:apple') AND status =
1 ORDER BY score() DESC LIMIT 10` can spend the per-segment top-k slots on
high-scoring `status = 0` rows, drop them later, and never return lower-scoring
`status = 1` rows that should be in the final top 10. Please either avoid the
top-k scorer path when other scan predicates remain, or pass the current
candidate bitmap into the scorer so the pushed limit is computed after all
pushed filters.
##########
regression-test/suites/search/test_search_score_cache.groovy:
##########
@@ -0,0 +1,240 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_search_score_cache", "p0") {
+ def tableName = "search_score_cache_test"
+
+ def assertPositiveScores = { result ->
+ assertTrue(result.size() > 0)
+ for (def row : result) {
+ assertTrue(Double.parseDouble(row[1].toString()) > 0.0)
+ }
+ }
+
+ def assertStablePositiveScoreQuery = { warmSql, scoreSql ->
+ def warmResult1 = sql warmSql
+ def warmResult2 = sql warmSql
+ assertEquals(warmResult1, warmResult2)
+
+ def scoreResult1 = sql scoreSql
+ def scoreResult2 = sql scoreSql
+ assertEquals(scoreResult1.size(), scoreResult2.size())
+ assertEquals(scoreResult1.collect { it[0] as int }.sort(),
+ scoreResult2.collect { it[0] as int }.sort())
+ assertPositiveScores(scoreResult1)
+ assertPositiveScores(scoreResult2)
+ }
+
+ sql "DROP TABLE IF EXISTS ${tableName}"
+ sql """
+ CREATE TABLE ${tableName} (
+ id INT,
+ status INT,
+ title VARCHAR(200),
+ content VARCHAR(500),
+ v VARIANT,
+ INDEX idx_title(title) USING INVERTED PROPERTIES(
+ "parser" = "english",
+ "support_phrase" = "true"
+ ),
+ INDEX idx_content(content) USING INVERTED PROPERTIES(
+ "parser" = "english",
+ "support_phrase" = "true"
+ ),
+ INDEX idx_v(v) USING INVERTED PROPERTIES(
+ "parser" = "english",
+ "support_phrase" = "true"
+ )
+ ) ENGINE=OLAP
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+
+ sql """INSERT INTO ${tableName} VALUES
+ (1, 1, 'apple banana cherry', 'red fruit sweet apple', '{"host":"apple
banana server"}'),
+ (2, 1, 'apple banana date', 'fresh apple banana salad',
'{"host":"apple server cluster"}'),
+ (3, 0, 'banana grape mango', 'yellow fruit tropical', '{"host":"banana
server"}'),
+ (4, 1, 'apple grape kiwi', 'green fruit fresh apple', '{"host":"green
apple server"}'),
+ (5, 0, 'mango pineapple coconut', 'tropical fruit exotic',
'{"host":"mango server"}'),
+ (6, 1, 'apple cherry plum', 'mixed fruit apple salad', '{"host":"apple
cherry node"}'),
+ (7, 0, 'banana coconut papaya', 'smoothie blend tropical',
'{"host":"banana coconut node"}'),
+ (8, 1, 'grape cherry apple', 'wine fruit tart apple', '{"host":"grape
apple node"}')
+ """
+ sql "sync"
+
+ sql """ set enable_common_expr_pushdown = true """
+ sql """ set enable_inverted_index_query_cache = true """
+
+ // SEARCH DSL + score() must execute scorers even when the DSL bitmap
cache is warm.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE search('title:apple')
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE search('title:apple')
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+ // MATCH_ANY + score() must bypass bitmap-only inverted index query cache.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple'
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple'
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+ // MATCH_ALL + score() should still produce collected BM25 scores after
cache warmup.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_ALL 'apple banana'
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_ALL 'apple banana'
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+ // MATCH_PHRASE + score() should not materialize default 0 scores from
cached bitmaps.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_PHRASE 'apple banana'
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_PHRASE 'apple banana'
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+
+ // score() with ordinary filters should still execute scorers after the
indexed bitmap cache is warm.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple' AND status = 1
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple' AND status = 1
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+ // score() on VARIANT subfields must not fall back to default 0 after
cache warmup.
+ assertStablePositiveScoreQuery(
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE CAST(v["host"] AS STRING) MATCH_PHRASE 'apple server'
+ ORDER BY id
+ """,
+ """
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE CAST(v["host"] AS STRING) MATCH_PHRASE 'apple server'
+ ORDER BY s DESC
+ LIMIT 10
+ """
+ )
+
+ // UNION ALL branches each have their own score() pushdown and must
collect non-zero scores.
+ def unionWarmSql = """
+ (
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple'
+ ORDER BY id
+ LIMIT 3
+ )
+ UNION ALL
+ (
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'banana'
+ ORDER BY id
+ LIMIT 3
+ )
+ """
+ def unionScoreSql = """
+ (
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'apple'
+ ORDER BY s DESC
+ LIMIT 3
+ )
+ UNION ALL
+ (
+ SELECT
/*+SET_VAR(enable_common_expr_pushdown=true,enable_inverted_index_query_cache=true)
*/
+ id, score() AS s
+ FROM ${tableName}
+ WHERE title MATCH_ANY 'banana'
+ ORDER BY s DESC
+ LIMIT 3
+ )
+ """
+ assertStablePositiveScoreQuery(unionWarmSql, unionScoreSql)
+
Review Comment:
This assertion can be flaky for the UNION case. The helper compares
`warmResult1` and `warmResult2` with full row-order equality, but
`unionWarmSql` has no top-level `ORDER BY`; the `ORDER BY` clauses inside each
UNION ALL branch only order each branch, not the final combined stream. If the
two branch streams are returned in a different order, this test fails even
though the row set and cache behavior are correct. Please either wrap the UNION
ALL in an outer query with an explicit branch/id ordering key, or normalize the
warm results before comparing 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]
