From d8995dc02a17eddbdb4639a0a18dcada897d2fc5 Mon Sep 17 00:00:00 2001 From: zz_y Date: Mon, 14 Sep 2026 14:17:35 -0600 Subject: [PATCH] fix: remove arbitrary physical candidate limits --- control_plane/src/physical/workload_cost.rs | 40 +++++++++++++++++-- .../materialization_candidates.rs | 34 ++++++++++------ 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/control_plane/src/physical/workload_cost.rs b/control_plane/src/physical/workload_cost.rs index eb535f5d..734bc040 100644 --- a/control_plane/src/physical/workload_cost.rs +++ b/control_plane/src/physical/workload_cost.rs @@ -640,10 +640,8 @@ fn select_candidates( frontend: super::compiler::QueryFrontend, ) -> Result { evidence.validate(&env)?; - if candidates.is_empty() || candidates.len() > 64 { - return Err(invalid( - "candidate inventory must contain 1..=64 alternatives", - )); + if candidates.is_empty() { + return Err(invalid("candidate inventory must not be empty")); } let candidate_key_sets: BTreeSet<_> = candidates .iter() @@ -1169,6 +1167,21 @@ mod tests { .compile_promql(candidate.clone(), environment.clone()) .unwrap(); assert!(identities.insert(plan.envelope.plan_id)); + // Each enabled leaf has a bound readout; disabled leaves must leave + // no local raw scan after exact-subtree externalization. + for entry in plan.query_plan.entries.values() { + assert_eq!(entry.materialization_bindings().len(), enabled); + assert!( + !entry.nodes.values().any(|node| matches!( + node, + crate::query_plan::QueryPlanNode::Logical { + operator: + asap_types::query_plan::logical::ResidualQueryOperator::Scan { .. }, + .. + } + )) + ); + } let cost = manifest(&plan, &candidate.queries).unwrap(); assert_eq!( cost.components @@ -1385,6 +1398,25 @@ mod tests { } } + // A cheaper quoted alternative after index 64 must still participate. + #[test] + fn selection_considers_candidates_beyond_64() { + let (candidates, env, mut evidence) = quoted(); + for cost in evidence.quotes[0].unit_costs.values_mut() { + *cost = 1e9; + } + let mut inventory = vec![candidates[0].clone(); 64]; + inventory.push(candidates[1].clone()); + let plan = select_lowest_cost_candidate(inventory, env, &evidence).unwrap(); + assert_eq!(plan.envelope.plan_id, evidence.quotes[1].manifest.plan_id); + let report = plan.cost_comparison.unwrap(); + assert_eq!(report.candidate_evaluations.len(), 65); + assert_eq!( + report.candidate_evaluations[64].status, + CandidateEvaluationStatus::Selected + ); + } + #[test] fn complete_cost_changes_selection_and_reports_shared_work_once() { let (candidates, env, mut evidence) = quoted(); diff --git a/control_plane/src/physical/workload_cost/materialization_candidates.rs b/control_plane/src/physical/workload_cost/materialization_candidates.rs index 2344192f..bc9bde6b 100644 --- a/control_plane/src/physical/workload_cost/materialization_candidates.rs +++ b/control_plane/src/physical/workload_cost/materialization_candidates.rs @@ -9,10 +9,10 @@ pub(super) struct MaterializationCandidateSets { pub eligible_materialization_count: usize, } -/// Enumerate every enabled_keys for up to four leaves. Larger forests retain all-materialized, -/// all-exact, then singleton/complement pairs in stable key order. The caller must -/// disclose bounded coverage; no unenumerated optimum is claimed. Reserve one -/// of the selector's 64 candidate slots for native execution. +/// Enumerate every subset for up to four optional materialization keys. Larger +/// forests retain all-materialized, all-exact, then every singleton/complement +/// pair in stable key order: 2 + 2N candidates instead of exponential search. +/// The caller must disclose non-exhaustive coverage; native execution is separate. pub(super) fn enumerate(keys: BTreeSet) -> MaterializationCandidateSets { let eligible_materialization_count = keys.len(); let ordered: Vec<_> = keys.iter().cloned().collect(); @@ -37,12 +37,8 @@ pub(super) fn enumerate(keys: BTreeSet) -> MaterializationCandidateSets BTreeSet::from([key.clone()]), keys.difference(&BTreeSet::from([key])).cloned().collect(), ] { - if candidate_key_sets.len() >= 63 { - break; - } - if !candidate_key_sets.contains(&enabled_keys) { - candidate_key_sets.push(enabled_keys); - } + // With more than four keys, these sets are all distinct. + candidate_key_sets.push(enabled_keys); } } } @@ -78,11 +74,23 @@ mod tests { .contains(&BTreeSet::from(["b".into()]))); } #[test] - fn large_inventory_reserves_native_slot_and_discloses_truncation() { + fn large_inventory_covers_every_singleton_and_complement() { let keys = (0..100).map(|i| format!("{i:03}")).collect(); let result = enumerate(keys); assert!(!result.exhaustive); - assert_eq!(result.candidate_key_sets.len(), 63); + assert_eq!(result.candidate_key_sets.len(), 202); + // Every leaf, including those beyond the old cutoff, gets both choices. + for key in &result.candidate_key_sets[0] { + assert!(result + .candidate_key_sets + .contains(&BTreeSet::from([key.clone()]))); + let complement = result.candidate_key_sets[0] + .iter() + .filter(|other| *other != key) + .cloned() + .collect(); + assert!(result.candidate_key_sets.contains(&complement)); + } assert_eq!(result.candidate_key_sets[0].len(), 100); assert!(result.candidate_key_sets[1].is_empty()); assert_eq!( @@ -91,7 +99,7 @@ mod tests { .iter() .collect::>() .len(), - 63 + 202 ); } #[test]