From 1b42ad609966b108ac1009c49c0c9c00803a029c Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Fri, 24 Jul 2026 21:16:06 +0300 Subject: [PATCH] mu-query: cleanups Cleanup the query running and handling the results. - in threading, use stable_sort - remove dead code - fix indentation in a few places - don't need Option in various run_... in mu-query.cc --- lib/mu-query-match-deciders.cc | 48 ++++++++----------- lib/mu-query-match-deciders.hh | 10 ++-- lib/mu-query-results.hh | 11 ++--- lib/mu-query-threads.cc | 88 ++++++++++++++++++---------------- lib/mu-query.cc | 54 +++++++++------------ lib/mu-query.hh | 8 ++-- 6 files changed, 102 insertions(+), 117 deletions(-) diff --git a/lib/mu-query-match-deciders.cc b/lib/mu-query-match-deciders.cc index 999d6093..2734e34b 100644 --- a/lib/mu-query-match-deciders.cc +++ b/lib/mu-query-match-deciders.cc @@ -1,5 +1,5 @@ /* -** Copyright (C) 2020-2022 Dirk-Jan C. Binnema +** Copyright (C) 2020-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify it ** under the terms of the GNU General Public License as published by the @@ -36,7 +36,8 @@ using namespace Mu; // We use the MatchDecider to gather information and use it for both queries. struct MatchDecider : public Xapian::MatchDecider { - MatchDecider(QueryFlags qflags, DeciderInfo& info) : qflags_{qflags}, decider_info_{info} {} + MatchDecider(QueryFlags qflags, DeciderInfo& info) : + qflags_{qflags}, decider_info_{info} {} /** * Update the match structure with unreadable/duplicate flags * @@ -44,13 +45,14 @@ struct MatchDecider : public Xapian::MatchDecider { * * @return a new QueryMatch object */ - QueryMatch make_query_match(const Xapian::Document& doc) const - { + QueryMatch make_query_match(const Xapian::Document& doc) const { QueryMatch qm{}; - auto msgid{opt_string(doc, Field::Id::MessageId) - .value_or(*opt_string(doc, Field::Id::Path))}; - if (!decider_info_.message_ids.emplace(std::move(msgid)).second) + // fall back to the (unique) path for message-id-less messages + auto msgid{opt_string(doc, Field::Id::MessageId)}; + if (!msgid) + msgid = opt_string(doc, Field::Id::Path); + if (msgid && !decider_info_.message_ids.emplace(std::move(*msgid)).second) qm.flags |= QueryMatch::Flags::Duplicate; const auto path{opt_string(doc, Field::Id::Path)}; @@ -67,8 +69,7 @@ struct MatchDecider : public Xapian::MatchDecider { * * @return true or false */ - bool should_include(const QueryMatch& qm) const - { + bool should_include(const QueryMatch& qm) const { if (any_of(qflags_ & QueryFlags::SkipDuplicates) && any_of(qm.flags & QueryMatch::Flags::Duplicate)) return false; @@ -79,19 +80,6 @@ struct MatchDecider : public Xapian::MatchDecider { return true; } - /** - * Gather thread ids from this match. - * - * @param doc the document (message) - * - */ - void gather_thread_ids(const Xapian::Document& doc) const - { - auto thread_id{opt_string(doc, Field::Id::ThreadId)}; - if (thread_id) - decider_info_.thread_ids.emplace(std::move(*thread_id)); - } - protected: const QueryFlags qflags_; DeciderInfo& decider_info_; @@ -194,8 +182,8 @@ Mu::make_related_decider(QueryFlags qflags, DeciderInfo& info) return std::make_unique(qflags, info); } -struct MatchDeciderThread final : public MatchDecider { - MatchDeciderThread(QueryFlags qflags, DeciderInfo& info) : MatchDecider{qflags, info} {} +struct MatchDeciderThread final : public Xapian::MatchDecider { + explicit MatchDeciderThread(const QueryMatches& matches): matches_{matches} {} /** * operator() * @@ -210,14 +198,16 @@ struct MatchDeciderThread final : public MatchDecider { */ bool operator()(const Xapian::Document& doc) const override { // we may have seen this match in the "Leader" query, - // or in the second (unbuounded) related query; - const auto it{decider_info_.matches.find(doc.get_docid())}; - return it != decider_info_.matches.end() && !it->second.thread_path.empty(); + // or in the second (unbounded) related query; + const auto it{matches_.find(doc.get_docid())}; + return it != matches_.end() && !it->second.thread_path.empty(); } +private: + const QueryMatches& matches_; }; std::unique_ptr -Mu::make_thread_decider(QueryFlags qflags, DeciderInfo& info) +Mu::make_thread_decider(const QueryMatches& matches) { - return std::make_unique(qflags, info); + return std::make_unique(matches); } diff --git a/lib/mu-query-match-deciders.hh b/lib/mu-query-match-deciders.hh index bd196059..4b70fcb4 100644 --- a/lib/mu-query-match-deciders.hh +++ b/lib/mu-query-match-deciders.hh @@ -1,5 +1,5 @@ /* -** Copyright (C) 2021-2024 Dirk-Jan C. Binnema +** Copyright (C) 2021-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify it ** under the terms of the GNU General Public License as published by the @@ -62,14 +62,14 @@ std::unique_ptr make_related_decider(QueryFlags qflags, De /** * Make a "thread" decider, that is, a MatchDecider that removes all but the - * document excepts for the ones found during initial/related searches. + * documents found during the initial/related searches (i.e., those that + * received a thread-path during threading). * - * @param qflags query flags - * @param match_info receives information about the matches. + * @param matches the matches found during the initial/related searches * * @return a unique_ptr to a match decider. */ -std::unique_ptr make_thread_decider(QueryFlags qflags, DeciderInfo& info); +std::unique_ptr make_thread_decider(const QueryMatches& matches); } // namespace Mu diff --git a/lib/mu-query-results.hh b/lib/mu-query-results.hh index b514d09c..ec160613 100644 --- a/lib/mu-query-results.hh +++ b/lib/mu-query-results.hh @@ -1,5 +1,5 @@ /* -** Copyright (C) 2022-2024 Dirk-Jan C. Binnema +** Copyright (C) 2022-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify it ** under the terms of the GNU General Public License as published by the @@ -21,12 +21,10 @@ #define MU_QUERY_RESULTS_HH__ #include -#include #include #include #include #include -#include #include #include #include @@ -192,6 +190,7 @@ public: QueryResultsIterator operator++(int) { auto old{mset_it_}; ++mset_it_; + mdoc_ = Nothing; return QueryResultsIterator{old, query_matches_}; } @@ -214,6 +213,7 @@ public: QueryResultsIterator operator--(int) { auto old{mset_it_}; --mset_it_; + mdoc_ = Nothing; return QueryResultsIterator{old, query_matches_}; } @@ -225,7 +225,6 @@ public: * @return true or false */ bool operator==(const QueryResultsIterator& rhs) const { return mset_it_ == rhs.mset_it_; } - bool operator!=(const QueryResultsIterator& rhs) const { return mset_it_ != rhs.mset_it_; } QueryResultsIterator& operator*() { return *this; } const QueryResultsIterator& operator*() const { return *this; } @@ -371,7 +370,7 @@ private: const Mu::Document& mu_document() const { if (!mdoc_) { if (auto xdoc = document(); !xdoc) - std::runtime_error("iter without document"); + throw std::runtime_error("iter without document"); else mdoc_ = Mu::Document{xdoc.value()}; } @@ -390,8 +389,6 @@ format_as(const QueryResultsIterator& it) return it.path().value_or(""); } -constexpr auto MaxQueryResultsSize = std::numeric_limits::max(); - class QueryResults { public: /// Helper types diff --git a/lib/mu-query-threads.cc b/lib/mu-query-threads.cc index ed6c7046..93fa4327 100644 --- a/lib/mu-query-threads.cc +++ b/lib/mu-query-threads.cc @@ -1,5 +1,5 @@ /* -** Copyright (C) 2022 Dirk-Jan C. Binnema +** Copyright (C) 2022-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify it ** under the terms of the GNU General Public License as published by the @@ -40,38 +40,29 @@ struct Container { Container(const Container&) = delete; Container(Container&&) = default; - void add_child(Container& new_child) - { + void add_child(Container& new_child) { new_child.parent = this; children.emplace_back(&new_child); } - void remove_child(Container& child) - { - children.erase(find_child(child)); + void remove_child(Container& child) { + std::erase(children, &child); assert(!has_child(child)); } - Containers::iterator find_child(Container& child) - { - return std::find_if(children.begin(), children.end(), [&](auto&& c) { - return c == &child; - }); + Containers::iterator find_child(Container& child) { + return std::ranges::find(children, &child); } - Containers::const_iterator find_child(Container& child) const - { - return std::find_if(children.begin(), children.end(), [&](auto&& c) { - return c == &child; - }); + Containers::const_iterator find_child(Container& child) const { + return std::ranges::find(children, &child); } bool has_child(Container& child) const { return find_child(child) != children.cend(); } - bool is_reachable(Container* other) const - { + bool is_reachable(Container* other) const { auto up{ur_parent()}; return up && up == other->ur_parent(); } - template void for_each_child(Func&& func) - { + + template void for_each_child(Func&& func) { auto it{children.rbegin()}; while (it != children.rend()) { auto next = std::next(it); @@ -137,15 +128,19 @@ handle_duplicates(IdTable& id_table, DupTable& dup_table) size_t n{}; for (auto&& dup : dup_table) { - const auto msgid{dup.first}; - auto it = id_table.find(msgid); + auto it = id_table.find(dup.first); if (it == id_table.end()) continue; - // add duplicates as fake children - char buf[32]; - ::snprintf(buf, sizeof(buf), "dup-%zu", ++n); - it->second.add_child(id_table.emplace(buf, std::move(dup.second)).first->second); + // add duplicates as fake children; hold a _reference_ to the + // parent, since the emplace below may invalidate the iterator + // (references remain valid). + auto& parent{it->second}; + auto& dup_child{id_table.emplace(mu_format("dup-{}", ++n), + std::move(dup.second)).first->second}; + if (dup_child.query_match) + dup_child.thread_date_key = dup_child.query_match->date_key; + parent.add_child(dup_child); } } @@ -156,11 +151,21 @@ determine_id_table(QueryResultsType& qres) // 1. For each query_match IdTable id_table; DupTable dups; + id_table.reserve(qres.size() * 2); // heuristic: msgids + references + for (auto&& mi : qres) { - const auto msgid{mi.message_id().value_or(*mi.path())}; + // fall back to the (unique) path for message-id-less messages + auto msgid_opt{mi.message_id()}; + if (!msgid_opt) + msgid_opt = mi.path(); + const auto msgid{std::move(msgid_opt).value_or(std::string{})}; + // Step 0 (non-JWZ): filter out dups, handle those at the end if (mi.query_match().has_flag(QueryMatch::Flags::Duplicate)) { - dups.emplace(msgid, mi.query_match()); + auto& qmatch{mi.query_match()}; + qmatch.date_key = mi.date_str().value_or(""); + qmatch.subject = mi.subject().value_or(""); + dups.emplace(msgid, qmatch); continue; } // 1.A If id_table contains an empty Container for this ID: @@ -357,7 +362,7 @@ inline std::string to_string(const ThreadPath& tpath, size_t digits) { std::string str; - str.reserve(tpath.size() * digits); + str.reserve(tpath.size() * (digits + 1)); // incl. ':' separators bool first{true}; for (auto&& segm : tpath) { @@ -369,19 +374,17 @@ to_string(const ThreadPath& tpath, size_t digits) } static bool // compare subjects, ignore anything before the last ':*' -subject_matches(const std::string& sub1, const std::string& sub2) +subject_matches(std::string_view sub1, std::string_view sub2) { - auto search_str = [](const std::string& s) -> const char* { + auto tail = [](std::string_view s) -> std::string_view { const auto pos = s.find_last_of(':'); - if (pos == std::string::npos) - return s.c_str(); - else { - const auto pos2 = s.find_first_not_of(' ', pos + 1); - return s.c_str() + (pos2 == std::string::npos ? pos : pos2); - } + if (pos == std::string_view::npos) + return s; + const auto pos2 = s.find_first_not_of(' ', pos + 1); + return s.substr(pos2 == std::string_view::npos ? pos : pos2); }; - return g_strcmp0(search_str(sub1), search_str(sub2)) == 0; + return tail(sub1) == tail(sub2); } static bool @@ -484,8 +487,10 @@ sort_container(Container& container) for (auto& child : container.children) sort_container(*child); - // now sort this level. - std::sort(container.children.begin(), container.children.end(), [&](auto&& c1, auto&& c2) { + // now sort this level; use a stable sort so messages with equal + // dates keep their original (mset) order. + std::stable_sort(container.children.begin(), container.children.end(), + [](auto&& c1, auto&& c2) { return c1->thread_date_key < c2->thread_date_key; }); @@ -522,7 +527,7 @@ sort_siblings(IdTable& id_table, bool descending) // // Note that unless we're testing, _xapian_ will handle // the ascending/descending of the top level. - std::sort(root_vec.begin(), root_vec.end(), [&](auto&& c1, auto&& c2) { + std::stable_sort(root_vec.begin(), root_vec.end(), [&](auto&& c1, auto&& c2) { #ifdef BUILD_TESTS if (descending) return c2->thread_date_key < c1->thread_date_key; @@ -946,6 +951,7 @@ try { g_test_add_func("/threader/thread-info/descending", test_thread_info_descending); return g_test_run(); + } catch (const std::runtime_error& re) { std::cerr << re.what() << "\n"; return 1; diff --git a/lib/mu-query.cc b/lib/mu-query.cc index 294ceb6c..cc4e3d2f 100644 --- a/lib/mu-query.cc +++ b/lib/mu-query.cc @@ -1,5 +1,5 @@ /* -** Copyright (C) 2008-2024 Dirk-Jan C. Binnema +** Copyright (C) 2008-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify ** it under the terms of the GNU General Public License as published by @@ -37,18 +37,18 @@ struct Query::Private { Field::Id sortfield_id, QueryFlags qflags) const; - Option run_threaded(QueryResults&& qres, Xapian::Enquire& enq, - QueryFlags qflags, size_t max_size) const; - Option run_singular(const std::string& expr, - Field::Id sortfield_id, - QueryFlags qflags, size_t maxnum) const; - Option run_related(const std::string& expr, - Field::Id sortfield_id, - QueryFlags qflags, size_t maxnum) const; + QueryResults run_threaded(QueryResults&& qres, Xapian::Enquire& enq, + QueryFlags qflags, size_t max_size) const; + QueryResults run_singular(const std::string& expr, + Field::Id sortfield_id, + QueryFlags qflags, size_t maxnum) const; + QueryResults run_related(const std::string& expr, + Field::Id sortfield_id, + QueryFlags qflags, size_t maxnum) const; - Option run(const std::string& expr, - Field::Id sortfield_id, QueryFlags qflags, - size_t maxnum) const; + QueryResults run(const std::string& expr, + Field::Id sortfield_id, QueryFlags qflags, + size_t maxnum) const; const Store& store_; const ParserFlags parser_flags_; }; @@ -121,7 +121,7 @@ struct ThreadKeyMaker : public Xapian::KeyMaker { const QueryMatches& match_info_; }; -Option +QueryResults Query::Private::run_threaded(QueryResults&& qres, Xapian::Enquire& enq, QueryFlags qflags, size_t maxnum) const { @@ -132,15 +132,14 @@ Query::Private::run_threaded(QueryResults&& qres, Xapian::Enquire& enq, QueryFla ThreadKeyMaker key_maker{qres.query_matches()}; enq.set_sort_by_key(&key_maker, descending); - DeciderInfo minfo; - minfo.matches = qres.query_matches(); - auto mset{enq.get_mset(0, maxnum, {}, make_thread_decider(qflags, minfo).get())}; + auto mset{enq.get_mset(0, maxnum, {}, + make_thread_decider(qres.query_matches()).get())}; mset.fetch(); return QueryResults{mset, std::move(qres.query_matches())}; } -Option +QueryResults Query::Private::run_singular(const std::string& expr, Field::Id sortfield_id, QueryFlags qflags, size_t maxnum) const @@ -155,11 +154,7 @@ Query::Private::run_singular(const std::string& expr, const auto threading{any_of(qflags & QueryFlags::Threading)}; DeciderInfo minfo{}; -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wextra" auto enq{make_enquire(expr, threading ? Field::Id::Date : sortfield_id, qflags)}; -#pragma GCC diagnostic ignored "-Wswitch-default" -#pragma GCC diagnostic pop auto mset{enq.get_mset(0, maxnum, {}, make_leader_decider(singular_qflags, minfo).get())}; mset.fetch(); @@ -181,7 +176,7 @@ opt_string(const Xapian::Document& doc, Field::Id id) noexcept return Some(std::move(val)); } -Option +QueryResults Query::Private::run_related(const std::string& expr, Field::Id sortfield_id, QueryFlags qflags, size_t maxnum) const @@ -228,7 +223,7 @@ Query::Private::run_related(const std::string& expr, return threading ? run_threaded(std::move(qres), r_enq, qflags, maxnum) : qres; } -Option +QueryResults Query::Private::run(const std::string& expr, Field::Id sortfield_id, QueryFlags qflags, size_t maxnum) const { @@ -257,11 +252,7 @@ Query::run(const std::string& expr, Field::Id sortfield_id, maxnum == 0 ? std::string{"∞"} : std::to_string(maxnum))}; return xapian_try_result([&]{ - if (auto&& res = priv_->run(expr, sortfield_id, qflags, maxnum); res) - return Result(Ok(std::move(res.value()))); - else - return Result(Err(Error::Code::Query, - "failed to run query")); + return Ok(priv_->run(expr, sortfield_id, qflags, maxnum)); }); } @@ -271,9 +262,10 @@ Query::count(const std::string& expr) const return xapian_try( [&] { const auto enq{priv_->make_enquire(expr, {}, {})}; - auto mset{enq.get_mset(0, priv_->store_.size())}; - mset.fetch(); - return mset.size(); + // with checkatleast == the db size, the "estimate" + // is exact; this avoids materializing the match items. + return enq.get_mset(0, 0, priv_->store_.size()) + .get_matches_estimated(); }, 0); } diff --git a/lib/mu-query.hh b/lib/mu-query.hh index fb3628bc..9a3abe8c 100644 --- a/lib/mu-query.hh +++ b/lib/mu-query.hh @@ -1,5 +1,5 @@ /* -** Copyright (C) 2008-2024 Dirk-Jan C. Binnema +** Copyright (C) 2008-2026 Dirk-Jan C. Binnema ** ** This program is free software; you can redistribute it and/or modify ** it under the terms of the GNU General Public License as published by @@ -17,8 +17,8 @@ ** */ -#ifndef __MU_QUERY_HH__ -#define __MU_QUERY_HH__ +#ifndef MU_QUERY_HH__ +#define MU_QUERY_HH__ #include #include @@ -91,4 +91,4 @@ private: }; } // namespace Mu -#endif /*__MU_QUERY_HH__*/ +#endif /*MU_QUERY_HH__*/