Skip to content

Commit 377a919

Browse files
authored
Merge pull request #37161 from vespa-engine/havardpe/fix-equiv-children-flow-stats
calculate flow stats for full children trees, not just one level
2 parents c56feb7 + 89a066a commit 377a919

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,29 @@ TEST(IntermediateBlueprintsTest, require_that_replaced_blueprints_retain_source_
10951095
EXPECT_EQ(42u, top2_up->getSourceId());
10961096
}
10971097

1098+
TEST(IntermediateBlueprintsTest, equiv_flow_stats_reach_into_intermediate_term_subtrees) {
1099+
// Equiv terms are not part of the optimize tree, so EquivBlueprint must
1100+
// update flow stats on the full subtree of each term — not just the term
1101+
// itself. Otherwise an IntermediateBlueprint sitting between Equiv and
1102+
// its real leaves (e.g. a SourceBlender wrapped around a term in proton)
1103+
// sees zero-initialized child stats, propagates estimate=0/cost=0 up to
1104+
// Equiv, and zeroes out the planner cost for everything that follows
1105+
// Equiv in an AND.
1106+
FieldSpecBaseList fields;
1107+
fields.add(FieldSpecBase(1, 1));
1108+
auto equiv = std::make_unique<EquivBlueprint>(fields, EquivBlueprint::allocate_outside_equiv_tag{});
1109+
for (uint32_t i = 0; i < 3; ++i) {
1110+
auto wrap = std::make_unique<OrBlueprint>();
1111+
wrap->addChild(ap(MyLeafSpec(40).addField(1, 2 + i * 2).create()));
1112+
wrap->addChild(ap(MyLeafSpec(40).addField(1, 3 + i * 2).create()));
1113+
equiv->addTerm(std::move(wrap), 1.0);
1114+
}
1115+
equiv->basic_plan(false, 1000);
1116+
EXPECT_GT(equiv->estimate(), 0.0);
1117+
EXPECT_GT(equiv->cost(), 0.0);
1118+
EXPECT_GT(equiv->strict_cost(), 0.0);
1119+
}
1120+
10981121
TEST(IntermediateBlueprintsTest, test_Equiv_Blueprint) {
10991122
FieldSpecBaseList fields;
11001123
search::fef::MatchDataLayout subLayout;

searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,9 @@ void EquivBlueprint::sort(InFlow in_flow) {
5757
}
5858

5959
FlowStats EquivBlueprint::calculate_flow_stats(uint32_t docid_limit) const {
60+
auto update = [docid_limit](Blueprint& bp) { bp.update_flow_stats(docid_limit); };
6061
for (auto& term : _terms) {
61-
term->update_flow_stats(docid_limit);
62+
term->each_node_post_order(update);
6263
}
6364
double est = OrFlow::estimate_of(_terms);
6465
return {est, OrFlow::cost_of(_terms, false), OrFlow::cost_of(_terms, true) + flow::heap_cost(est, _terms.size())};

0 commit comments

Comments
 (0)