diff options
Diffstat (limited to 'searchcore')
5 files changed, 35 insertions, 9 deletions
diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index a4952fa3154..d690fb29795 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -785,8 +785,21 @@ TEST("require that match params are set up straight with ranking on") { ASSERT_EQUAL(0.7, p.rankDropLimit); ASSERT_EQUAL(0u, p.offset); ASSERT_EQUAL(1u, p.hits); + ASSERT_TRUE(p.has_rank_drop_limit()); } +TEST("require that match params can turn off rank-drop-limit") { + MatchParams p(1, 2, 4, -std::numeric_limits<feature_t>::quiet_NaN(), 0, 1, true, true); + ASSERT_EQUAL(1u, p.numDocs); + ASSERT_EQUAL(2u, p.heapSize); + ASSERT_EQUAL(4u, p.arraySize); + ASSERT_TRUE(std::isnan(p.rankDropLimit)); + ASSERT_EQUAL(0u, p.offset); + ASSERT_EQUAL(1u, p.hits); + ASSERT_FALSE(p.has_rank_drop_limit()); +} + + TEST("require that match params are set up straight with ranking on arraySize is atleast the size of heapSize") { MatchParams p(1, 6, 4, 0.7, 1, 1, true, true); ASSERT_EQUAL(1u, p.numDocs); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp index 983e09716c3..7465088d8f0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_params.cpp @@ -2,6 +2,7 @@ #include "match_params.h" #include <algorithm> +#include <cmath> namespace proton::matching { @@ -32,4 +33,8 @@ MatchParams::MatchParams(uint32_t numDocs_in, rankDropLimit(rankDropLimit_in) { } +bool MatchParams::has_rank_drop_limit() const { + return ! std::isnan(rankDropLimit); +} + } diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_params.h b/searchcore/src/vespa/searchcore/proton/matching/match_params.h index 40f33e25914..efcd507c9af 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_params.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_params.h @@ -28,6 +28,7 @@ struct MatchParams { bool hasFinalRank, bool needRanking=true); bool save_rank_scores() const { return ((heapSize + arraySize) != 0); } + bool has_rank_drop_limit() const; }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp index 87b4f918d11..c9ec2a06353 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp @@ -10,7 +10,6 @@ #include <vespa/searchlib/common/bitvector.h> #include <vespa/searchlib/queryeval/multibitvectoriterator.h> #include <vespa/searchlib/queryeval/andnotsearch.h> -#include <vespa/vespalib/util/thread_bundle.h> #include <vespa/vespalib/data/slime/cursor.h> #include <vespa/vespalib/data/slime/inserter.h> @@ -234,10 +233,10 @@ template <bool do_rank, bool do_limit, bool do_share> void MatchThread::match_loop_helper_rank_limit_share(MatchTools &tools, HitCollector &hits) { - if (std::isnan(matchParams.rankDropLimit)) { - match_loop_helper_rank_limit_share_drop<do_rank, do_limit, do_share, false>(tools, hits); - } else { + if (matchParams.has_rank_drop_limit()) { match_loop_helper_rank_limit_share_drop<do_rank, do_limit, do_share, true>(tools, hits); + } else { + match_loop_helper_rank_limit_share_drop<do_rank, do_limit, do_share, false>(tools, hits); } } @@ -376,8 +375,14 @@ MatchThread::processResult(const Doom & doom, } } } - if (auto onMatchTask = matchToolsFactory.createOnMatchTask()) { - onMatchTask->run(search::ResultSet::stealResult(std::move(*result))); + + if (auto task = matchToolsFactory.createOnMatchTask()) { + // This is not correct, as it should use the results before rank-drop-limit + // But keeping like this for now as on-first-phase should be a subset of -on-match + task->run(result->copyResult()); + } + if (auto task = matchToolsFactory.createOnFirstPhaseTask()) { + task->run(search::ResultSet::stealResult(std::move(*result))); } if (hasGrouping) { context.grouping->setDistributionKey(_distributionKey); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 9506ca99b66..d652435cbca 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -253,13 +253,15 @@ MatchToolsFactory::createTask(vespalib::stringref attribute, vespalib::stringref std::unique_ptr<AttributeOperationTask> MatchToolsFactory::createOnMatchTask() const { const auto & op = _rankSetup.getMutateOnMatch(); - return createTask(execute::onmatch::Attribute::lookup(_queryEnv.getProperties(), op._attribute), - execute::onmatch::Operation::lookup(_queryEnv.getProperties(), op._operation)); + return createTask(op._attribute, op._operation); } std::unique_ptr<AttributeOperationTask> MatchToolsFactory::createOnFirstPhaseTask() const { const auto & op = _rankSetup.getMutateOnFirstPhase(); - return createTask(op._attribute, op._operation); + // Note that combining onmatch in query with first-phase is not a bug. + // It is intentional, as the semantics of onmatch in query are identical to on-first-phase. + return createTask(execute::onmatch::Attribute::lookup(_queryEnv.getProperties(), op._attribute), + execute::onmatch::Operation::lookup(_queryEnv.getProperties(), op._operation)); } std::unique_ptr<AttributeOperationTask> MatchToolsFactory::createOnSecondPhaseTask() const { |