airborne12 commented on code in PR #68758: URL: https://github.com/apache/doris/pull/68758#discussion_r4266548365
########## be/src/exprs/function/search/function_search.cpp: ########## @@ -0,0 +1,634 @@ +// 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. + +#include "exprs/function/search/function_search.h" + +#include <CLucene/config/repl_wchar.h> +#include <CLucene/debug/error.h> +#include <fmt/format.h> +#include <gen_cpp/Exprs_types.h> +#include <glog/logging.h> + +#include <algorithm> +#include <iterator> +#include <memory> +#include <optional> +#include <roaring/roaring.hh> +#include <string> +#include <unordered_map> +#include <utility> +#include <vector> + +#include "common/config.h" +#include "common/exception.h" +#include "common/status.h" +#include "core/block/columns_with_type_and_name.h" +#include "exprs/function/search/function_search_leaf.h" +#include "exprs/function/search/search_lowering.h" +#include "exprs/function/search/variant_inverted_index_search.h" +#include "exprs/function/simple_function_factory.h" +#include "exprs/vexpr_context.h" +#include "runtime/runtime_profile.h" +#include "storage/index/index_file_reader.h" +#include "storage/index/index_query_context.h" +#include "storage/index/inverted/analysis/analyzer/analyzer.h" +#include "storage/index/inverted/inverted_index_iterator.h" +#include "storage/index/inverted/inverted_index_reader.h" +#include "storage/index/inverted/query/engine/all_query/all_query.h" +#include "storage/index/inverted/query/engine/bit_set_query/bit_set_query.h" +#include "storage/index/inverted/query/engine/boolean_query/boolean_query_builder.h" +#include "storage/index/inverted/query/engine/boolean_query/operator.h" +#include "storage/index/inverted/query/engine/collect/doc_set_collector.h" +#include "storage/index/inverted/query/engine/collect/top_k_collector.h" +#include "storage/index/inverted/query/exec/term_pattern.h" +#include "storage/index/inverted/query/logical/node.h" +#include "storage/index/inverted/reader/collection_statistics.h" +#include "storage/olap_common.h" +#include "storage/segment/variant/nested_group_provider.h" +#include "util/thrift_util.h" + +namespace doris { + +namespace query_v2 = segment_v2::inverted_index::query_v2; + +// Serializes the whole TSearchParam, so every field of the DSL takes part in the signature, and +// adds the limit on the terms PREFIX, WILDCARD and REGEXP clauses expand to. +std::string FunctionSearch::dsl_cache_signature(const TSearchParam& search_param, + const IndexQueryContext* context) { + ThriftSerializer ser(false, 1024); + TSearchParam copy = search_param; + std::string sig; + auto st = ser.serialize(©, &sig); + if (UNLIKELY(!st.ok())) { + LOG(WARNING) << "build_dsl_signature: Thrift serialization failed: " << st.to_string() + << ", caching disabled for this query"; + return ""; + } + sig += "|max_expansions="; + sig += std::to_string( + index_query::max_expansions(context != nullptr ? context->runtime_state : nullptr)); + return sig; +} + +// Extract segment path prefix from the first available inverted index iterator. +// All fields in the same segment share the same path prefix. +static std::string extract_segment_prefix( + const std::unordered_map<std::string, IndexIterator*>& iterators) { + for (const auto& [field_name, iter] : iterators) { + auto* inv_iter = dynamic_cast<InvertedIndexIterator*>(iter); + if (!inv_iter) continue; + // Try fulltext reader first, then string type + for (auto type : + {InvertedIndexReaderType::FULLTEXT, InvertedIndexReaderType::STRING_TYPE}) { + IndexReaderType reader_type = type; + auto reader = inv_iter->get_reader(reader_type); + if (!reader) continue; + auto inv_reader = std::dynamic_pointer_cast<InvertedIndexReader>(reader); + if (!inv_reader) continue; + auto file_reader = inv_reader->get_index_file_reader(); + if (!file_reader) continue; + return file_reader->get_index_path_prefix(); + } + } + VLOG_DEBUG << "extract_segment_prefix: no suitable inverted index reader found across " + << iterators.size() << " iterators, caching disabled for this query"; + return ""; +} + +namespace { + +bool is_nested_group_search_supported() { + auto provider = segment_v2::create_nested_group_read_provider(); + return provider != nullptr && provider->should_enable_nested_group_read_path(); +} + +namespace logical = index_query::logical; + +// Lowering's view of the resolver: what index a field binds to and how that +// index analyzes values. +class SearchFieldCatalog final : public logical::FieldCatalog { +public: + SearchFieldCatalog(FieldReaderResolver& resolver, std::shared_ptr<IndexQueryContext> context) + : _resolver(resolver), _context(std::move(context)) {} + + Status resolve(const std::string& field, InvertedIndexQueryType query_type, + logical::FieldProps* out) override { + FieldReaderBinding binding; + RETURN_IF_ERROR(_resolver.resolve(field, query_type, &binding)); + if (!binding.is_bound()) { + LOG(INFO) << "search: No inverted index for field '" << field + << "' in this segment, query_type=" << static_cast<int>(query_type) + << ", returning UNKNOWN bitmap"; + *out = logical::FieldProps {}; + return Status::OK(); + } + const bool scalar = binding.inverted_reader != nullptr && + binding.inverted_reader->type() == InvertedIndexReaderType::BKD; + const bool analyzed = !scalar && inverted_index::InvertedIndexAnalyzer::should_analyzer( + binding.index_properties); + *out = logical::FieldProps {.bound = true, + .direct_index = scalar, + .analyzed = analyzed, + .binding = binding.binding_key}; + return Status::OK(); + } + + Status analyze(const logical::FieldProps& props, const std::string& value, + std::vector<logical::Token>* out) override { + const FieldReaderBinding* binding = _resolver.find_binding(props.binding); + if (binding == nullptr) { + return Status::InternalError("search: no binding '{}' to analyze with", props.binding); + } + int64_t unused_timer = 0; + SCOPED_RAW_TIMER(_context != nullptr && _context->stats != nullptr + ? &_context->stats->inverted_index_analyzer_timer + : &unused_timer); + InvertedIndexAnalyzerCtxSPtr analyzer_ctx; + RETURN_IF_ERROR(_resolver.analyzer_context_for(props.binding, &analyzer_ctx)); + return inverted_index::InvertedIndexAnalyzer::analyze(value, analyzer_ctx.get(), + binding->index_properties, out); + } + + Status normalize(const logical::FieldProps& props, const std::string& value, + std::string* out) override { + const FieldReaderBinding* binding = _resolver.find_binding(props.binding); + if (binding == nullptr) { + return Status::InternalError("search: no binding '{}' to normalize with", + props.binding); + } + try { + *out = inverted_index::InvertedIndexAnalyzer::normalize(value, + binding->index_properties); + } catch (const CLuceneError& e) { + return Status::Error<ErrorCode::INVERTED_INDEX_ANALYZER_ERROR>( + "search: normalizing '{}' failed: {}", value, e.what()); + } catch (const Exception& e) { + return Status::Error<ErrorCode::INVERTED_INDEX_ANALYZER_ERROR>( + "search: normalizing '{}' failed: {}", value, e.what()); + } + return Status::OK(); + } + +private: + FieldReaderResolver& _resolver; + std::shared_ptr<IndexQueryContext> _context; +}; + +query_v2::Occur to_query_occur(logical::Occur occur) { + switch (occur) { + case logical::Occur::kShould: + return query_v2::Occur::SHOULD; + case logical::Occur::kMustNot: + return query_v2::Occur::MUST_NOT; + case logical::Occur::kMust: + default: + return query_v2::Occur::MUST; + } +} + +Status compile_node(const logical::Node& node, const SearchLeafContext& ctx, + FieldReaderResolver& resolver, query_v2::QueryPtr* out, + std::string* binding_key); + +using Clauses = std::vector<std::pair<logical::Occur, logical::NodePtr>>; + +bool has_required_clause(const Clauses& clauses) { + return std::ranges::any_of( + clauses, [](const auto& clause) { return clause.first == logical::Occur::kMust; }); +} + +// Without scoring, optional clauses beside a required one change neither the rows nor their +// UNKNOWN state, so they are left out. +Clauses matching_clauses(const logical::Bool& boolean, bool scoring) { + if (scoring || boolean.op != logical::BoolOp::kOccur || boolean.min_should_match != 0 || + !has_required_clause(boolean.clauses)) { + return boolean.clauses; + } + Clauses kept; + std::ranges::copy_if(boolean.clauses, std::back_inserter(kept), [](const auto& clause) { + return clause.first != logical::Occur::kShould; + }); + return kept; +} + +// AND, OR and NOT ignore the per-clause occur; OCCUR keeps it and the threshold. A Boolean +// left with one clause after dropping clauses that only score is that clause. +Status compile_bool(const logical::Bool& boolean, const SearchLeafContext& ctx, + FieldReaderResolver& resolver, query_v2::QueryPtr* out) { + const Clauses clauses = matching_clauses(boolean, ctx.scoring); + if (clauses.size() == 1 && boolean.clauses.size() > 1) { + return compile_node(*clauses.front().second, ctx, resolver, out, nullptr); Review Comment: Fixed in 9173b5763ba. `compile_bool` now passes the binding key of the clause it keeps to its caller, so a Boolean left with one clause reads that clause's index. Before, it compiled the clause without a key, and the term fell back to the index registered last for the field. `TestCollapsedBooleanKeepsTheIndexOfItsClause` gives `body` a keyword index holding `Foo` and an analyzed index holding `foo`, then runs this query unscored: `EXACT(Foo)` as MUST, `baz` and `bar` as SHOULD. It returned no rows before the change and returns row 0 now. ########## be/test/storage/index/inverted/query/phrase/sloppy_phrase_matcher_test.cpp: ########## @@ -210,27 +209,23 @@ TEST(SniiSloppyPhraseMatcher, GeneratedCasesMatchV3Oracle) { plan_index.back() = plan_index.front(); positions.back() = positions.front(); } else if (iteration % 3 == 2) { - std::fill(plan_index.begin(), plan_index.end(), 0); - std::fill(positions.begin(), positions.end(), positions.front()); + std::ranges::fill(plan_index, 0); + std::ranges::fill(positions, positions.front()); } std::vector<uint32_t> offsets(clause_count); std::iota(offsets.begin(), offsets.end(), 0U); const auto spans = make_spans(positions); for (uint32_t slop : {1U, 3U, 7U}) { - SCOPED_TRACE(::testing::Message() << "iteration=" << iteration - << " clauses=" << clause_count << " slop=" << slop); SloppyPhraseMatcher unordered(plan_index, offsets, slop, false); - EXPECT_FLOAT_EQ( - unordered.match(spans, true), - v3_unordered_frequency(plan_index, positions, static_cast<int32_t>(slop))); - + mix(unordered.match(spans, true)); SloppyPhraseMatcher ordered(plan_index, offsets, slop, true); - EXPECT_FLOAT_EQ(ordered.match(spans, true), - v3_ordered_frequency(positions, static_cast<int32_t>(slop))); + mix(ordered.match(spans, true)); } } + EXPECT_EQ(count, 1536U); + EXPECT_EQ(digest, 8055439586674593017ULL); Review Comment: Fixed in 802d64a4eb9, keeping the digest. The test now draws its bounded values with a copy of the method libstdc++ uses (Lemire's nearly divisionless draw from a 32-bit generator) instead of `std::uniform_int_distribution`. Every standard library therefore generates the cases the digest was computed from. A standalone check with the test's seed and ranges confirms it, using 100,000 draws each for [1,5], [1,4] and [2,4]. Built with libc++, `std::uniform_int_distribution` differs from the first draw, while the copied method gives the libstdc++ sequence with both libraries. `GeneratedCasesMatchV3Digest` passes with the unchanged digest. -- 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]
