github-actions[bot] commented on code in PR #67180:
URL: https://github.com/apache/doris/pull/67180#discussion_r4092881505
##########
regression-test/suites/inverted_index_p0/cloud_build_index/test_cloud_build_index_update.groovy:
##########
@@ -91,7 +91,7 @@ suite("test_cloud_build_index_update") {
sql """build index on test_cloud_build_idx_uq_table"""
wait_for_last_build_index_finish("test_cloud_build_idx_uq_table", timeout)
check_inverted_index_filter_rows("select * from
test_cloud_build_idx_uq_table where address='hhhhh'" +
- " order by user_id,username,age,address", 12)
+ " order by user_id,username,age,address", 4)
Review Comment:
[P1] Restore the non-phrase filter count after the final ordering gate. This
query is a plain equality predicate, so `can_consume_candidate_rows()` is false
and the final `prune_before_index` gate leaves scan/delete restrictions after
inverted-index accounting. That restores the base metric ordering, where this
assertion was 12; expecting 4 reflects the earlier unconditional reorder and
should make this Cloud P0 case fail at the final head. Please keep the expected
count at 12 (or change the query to a candidate-consuming phrase predicate if
this case is meant to exercise early pruning).
##########
be/src/common/config.cpp:
##########
@@ -1320,6 +1322,18 @@ DEFINE_Bool(enable_inverted_index_cache_check_timestamp,
"true");
DEFINE_mBool(enable_inverted_index_correct_term_write, "true");
DEFINE_Int32(inverted_index_fd_number_limit_percent, "20"); // 20%
DEFINE_Int32(inverted_index_query_cache_shards, "256");
+DEFINE_mDouble(inverted_index_candidate_pushdown_ratio, "0.3");
+DEFINE_Validator(inverted_index_candidate_pushdown_ratio,
+ [](const double v) -> bool { return std::isfinite(v) && v <=
1.0; });
+static std::atomic<double> published_inverted_index_candidate_pushdown_ratio
{0.0};
+static std::mutex inverted_index_candidate_pushdown_ratio_update_lock;
+DEFINE_ON_UPDATE(inverted_index_candidate_pushdown_ratio, [](double, double
value) {
Review Comment:
[P2] Keep failed persistent updates coherent with the scan-side value.
`UPDATE_FIELD` assigns the raw ratio and `full_conf_map` before calling
`persist_config()`, but a persistence error returns before this callback runs.
A request such as `persist=true` against an unwritable custom config therefore
reports `BAD` and SHOW CONFIG exposes the new ratio, while every scan continues
reading the old atomic value indefinitely. Please either roll back the earlier
mutations on persistence failure or publish a single coherent in-memory state,
and cover this exit in the config test.
##########
be/src/storage/index/snii/query/phrase_prefix_exec.cpp:
##########
@@ -493,12 +494,56 @@ Status collect_merged_tail_matches(const
LogicalIndexReader& idx,
return Status::OK();
}
+// Picks how the leading phrase of a multi-tail prefix query is restricted. A
tail union much
+// smaller than the leading candidate set prefilters it; otherwise scan
candidates, when present,
+// restrict it directly. The union is kept whenever the unrestricted query
would use it, so scan
+// candidates never make the leading phrase costlier; they can also enable it,
since they bound
+// the leading positions to decode.
+Status restrict_prefix_leading_phrase(const LogicalIndexReader& idx,
+ const internal::ResolvedPhrasePlan&
exact_plan,
+ const std::vector<ResolvedQueryTerm>&
tail_terms,
+ const roaring::Roaring* candidates,
+ std::vector<uint32_t>* storage,
+ CandidateRestriction* restriction) {
+ uint32_t min_lead_df = std::numeric_limits<uint32_t>::max();
+ for (const ResolvedQueryTerm& term : exact_plan.unique_terms) {
+ min_lead_df = std::min(min_lead_df, term.entry.df);
+ }
+ uint64_t tail_df_sum = 0;
+ for (const ResolvedQueryTerm& tail : tail_terms) {
+ tail_df_sum += tail.entry.df;
+ }
+ const bool union_pays_off_unrestricted =
+ min_lead_df >= prefix_leading_prefilter_min_df(
+ idx, exact_plan.phrase_plan_index.size() ==
1) &&
+ tail_df_sum <= min_lead_df / kPrefixLeadingToTailDfRatio;
+ const uint64_t lead_bound =
+ candidates == nullptr ? 0 : std::min<uint64_t>(min_lead_df,
candidates->cardinality());
+ const bool union_pays_off_restricted = lead_bound >=
kMinPrefixLeadingPrefilterMinDf &&
+ tail_df_sum <= lead_bound /
kPrefixLeadingToTailDfRatio;
+ if (!union_pays_off_unrestricted && !union_pays_off_restricted) {
Review Comment:
[P2] Let selective candidates bypass the full prefix-tail union. When
candidates are present, this OR still honors `union_pays_off_unrestricted` even
if the candidate-aware bound says the union cannot pay off. For example, lead
df 1M, tail df sum 100k, and one candidate builds and decodes the entire 100k
tail union, intersects it to one row, and then replans/decodes those tail
postings during verification; directly restricting the leading phrase to that
one candidate avoids the preliminary union. Please use the candidate-aware
decision whenever `candidates != nullptr` (keeping the unrestricted decision
only without candidates) and add a selective-candidate counter/benchmark case
alongside the dense-candidate test.
--
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]
