airborne12 commented on code in PR #67180:
URL: https://github.com/apache/doris/pull/67180#discussion_r4093077008
##########
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:
Confirmed with a failing ASAN unit test: a persistent update from 0.2 to 0.4
returned an error but left the registered value and SHOW CONFIG map at 0.4
while scans read 0.2. Commit a99f9cb7a5eb0ceb698d3b4621fe057db5b397e4 rolls
back the registered value on persistence failure and updates the map only after
persistence succeeds. The test now passes, along with 30 other related ASAN
tests.
##########
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:
Confirmed. With one candidate, the unrestricted cost decision still built
the full tail union. In the deterministic ASAN IO test, the restricted query
read 3959 bytes before the fix; commit a99f9cb7a5eb0ceb698d3b4621fe057db5b397e4
uses the candidate-aware decision and reads 2520 bytes. The 10x read bound now
passes, and dense-candidate and result-equivalence tests also pass.
--
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]
