From a92c24b6c3d2f0e0ed3b2611919c665296244210 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:30:13 +0100 Subject: [PATCH 1/2] fix(planner): carry projections and early_limit into the physical plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PlanNode` has four fields; `PlanStep` had six, and neither `projections` nor `early_limit` was among them. The optimizer computed both, then dropped them at the logical -> physical boundary, so the executor never saw either pushdown. That is worse than merely lossy, and this is the part worth flagging. `CostModel::estimate` already *discounts* a node carrying an early limit (`cost.rs:345-350`): if let Some(limit) = node.early_limit { let limit_factor = (limit as f64 / 1000.0).min(1.0); selectivity *= limit_factor; time_ms *= 0.5 + 0.5 * limit_factor; // At least 50% of base cost } So a plan was priced as cheap as half its base cost *because of* a limit pushdown that the physical plan had no way to perform. The planner was rewarding an optimisation it then discarded. Nothing failed loudly — plans simply cost less on paper than they cost to run, which is the kind of defect that shows up as unexplained latency rather than as an error. Measured before the change: `early_limit` had exactly one non-test read in the entire tree — the cost adjustment above. `projections` had **none**; nothing read it after `vcl_bridge.rs` built it. Both fields are now on `PlanStep` and populated from the node. They carry `#[serde(default)]`, so a `PhysicalPlan` serialised before this change still deserialises (absent -> `vec![]` / `None`, matching the old behaviour rather than inventing a limit). This surfaced while discharging `optimize_is_permutation` (#206). The Coq model abstracts a node as `(modality, list condition)` — precisely the projection that omits the two fields being lost. The abstraction was not wrong, but it was drawn around the bug, which is why proving the permutation property could not have caught it. Test: `test_pushdown_survives_optimization` asserts both fields survive, and matches steps by modality rather than index — asserting on `steps[0]` would test the sort order, not the pushdown. Verified as a real guard, not a vacuous one: with the population reverted it fails with `projection pushdown was dropped at the physical boundary`; with the fix it passes. Verified: `cargo test -p verisim-planner` 118 passed / 0 failed; `cargo clippy -p verisim-planner --all-targets` clean; `cargo check -p verisim-api --all-targets` clean (it maps `PlanStep` into its own GraphQL type); `reuse lint` compliant. Not addressed here, and deliberately so: `pushed_predicates` remains `Vec` built by `format!("{:?}", c)`, so conditions still cross the boundary as Rust `Debug` output rather than as `ConditionKind`. Retyping it is a larger change with its own serialisation surface, and it is not needed to stop the cost model lying. --- rust-core/verisim-planner/src/explain.rs | 6 ++ rust-core/verisim-planner/src/optimizer.rs | 73 +++++++++++++++++++++ rust-core/verisim-planner/src/plan.rs | 20 ++++++ rust-core/verisim-planner/src/prepared.rs | 2 + rust-core/verisim-planner/src/profiler.rs | 4 ++ rust-core/verisim-planner/src/slow_query.rs | 2 + 6 files changed, 107 insertions(+) diff --git a/rust-core/verisim-planner/src/explain.rs b/rust-core/verisim-planner/src/explain.rs index 95153a48..d87c448e 100644 --- a/rust-core/verisim-planner/src/explain.rs +++ b/rust-core/verisim-planner/src/explain.rs @@ -254,6 +254,8 @@ mod tests { }, optimization_hint: Some("HNSW ANN search (k=10)".to_string()), pushed_predicates: vec!["Similarity { k: 10 }".to_string()], + projections: vec![], + early_limit: None, }, PlanStep { step: 2, @@ -266,6 +268,8 @@ mod tests { io_cost: 135.0, cpu_cost: 90.0, }, + projections: vec![], + early_limit: None, optimization_hint: Some("Graph traversal: relates_to (depth=2)".to_string()), pushed_predicates: vec!["Traversal { predicate: relates_to }".to_string()], }, @@ -334,6 +338,8 @@ mod tests { }, optimization_hint: Some("ZKP verification — expensive".to_string()), pushed_predicates: vec![], + projections: vec![], + early_limit: None, }], strategy: ExecutionStrategy::Sequential, total_cost: CostEstimate { diff --git a/rust-core/verisim-planner/src/optimizer.rs b/rust-core/verisim-planner/src/optimizer.rs index 1953dac8..dfc986e9 100644 --- a/rust-core/verisim-planner/src/optimizer.rs +++ b/rust-core/verisim-planner/src/optimizer.rs @@ -125,6 +125,12 @@ impl Planner { cost: cost.clone(), optimization_hint: hint.clone(), pushed_predicates, + // Carry both through to the physical plan. Dropping them here + // was not merely lossy: CostModel::estimate discounts a node + // that has an early_limit, so the plan was priced for a + // pushdown the executor could never see. + projections: node.projections.clone(), + early_limit: node.early_limit, }); cost_estimates.push(cost.clone()); @@ -352,4 +358,71 @@ mod tests { assert!(explain.text_output.contains("Strategy")); assert!(explain.text_output.contains("vector")); } + + /// Projection and early-limit pushdown must survive optimization. + /// + /// Before 2026-07-28 both were computed on the logical node and dropped at + /// the physical boundary. That was worse than lossy: `CostModel::estimate` + /// discounts a node carrying an `early_limit` (scaling `selectivity` and + /// taking `time_ms` to as low as 50% of base), so plans were priced for a + /// pushdown the executor could not see. Nothing failed loudly — the plan + /// just quietly cost less than it ran. + #[test] + fn test_pushdown_survives_optimization() { + let logical = LogicalPlan { + source: QuerySource::Octad, + nodes: vec![ + PlanNode { + modality: Modality::Vector, + conditions: vec![ConditionKind::Similarity { k: 10 }], + projections: vec!["id".to_string(), "label".to_string()], + early_limit: Some(25), + }, + PlanNode { + modality: Modality::Graph, + conditions: vec![ConditionKind::Traversal { + predicate: "relates_to".to_string(), + depth: Some(2), + }], + projections: vec!["id".to_string()], + early_limit: None, + }, + ], + post_processing: vec![], + }; + + let planner = Planner::new(PlannerConfig::default()); + let physical = planner.optimize(&logical).expect("optimize"); + + // Match on modality rather than position: the optimizer reorders by + // execution priority, so asserting on steps[0] would be asserting on + // the sort order, not on the pushdown. + let vector_step = physical + .steps + .iter() + .find(|s| s.modality == Modality::Vector) + .expect("vector step present"); + assert_eq!( + vector_step.projections, + vec!["id".to_string(), "label".to_string()], + "projection pushdown was dropped at the physical boundary" + ); + assert_eq!( + vector_step.early_limit, + Some(25), + "early-limit pushdown was dropped, but the cost model already \ + discounted the plan for it" + ); + + let graph_step = physical + .steps + .iter() + .find(|s| s.modality == Modality::Graph) + .expect("graph step present"); + assert_eq!(graph_step.projections, vec!["id".to_string()]); + assert_eq!( + graph_step.early_limit, None, + "a node without an early limit must not acquire one" + ); + } } diff --git a/rust-core/verisim-planner/src/plan.rs b/rust-core/verisim-planner/src/plan.rs index 3ec177b6..1d6ae8d3 100644 --- a/rust-core/verisim-planner/src/plan.rs +++ b/rust-core/verisim-planner/src/plan.rs @@ -113,6 +113,24 @@ pub struct PlanStep { pub optimization_hint: Option, /// Pushed-down predicates. pub pushed_predicates: Vec, + /// Fields to project, carried through from [`PlanNode::projections`] + /// (empty = all fields). + /// + /// Until 2026-07-28 this was absent, so projection pushdown was computed + /// on the logical node and then discarded at the physical boundary — the + /// executor had no way to see it. + #[serde(default)] + pub projections: Vec, + /// Early row limit pushed down to the store, carried through from + /// [`PlanNode::early_limit`]. + /// + /// Also absent until 2026-07-28, which was the more consequential half of + /// the same gap: [`CostModel::estimate`] *already* discounts a node + /// carrying an early limit (it scales `selectivity` and takes `time_ms` + /// down to as low as 50% of base), so a plan was scored cheaper for a + /// pushdown the physical plan could not actually perform. + #[serde(default)] + pub early_limit: Option, } /// An optimized physical plan ready for execution. @@ -182,6 +200,8 @@ mod tests { }, optimization_hint: Some("HNSW ANN".to_string()), pushed_predicates: vec!["k=10".to_string()], + projections: vec![], + early_limit: None, }], strategy: ExecutionStrategy::Sequential, total_cost: CostEstimate { diff --git a/rust-core/verisim-planner/src/prepared.rs b/rust-core/verisim-planner/src/prepared.rs index df8b8b4b..966709c4 100644 --- a/rust-core/verisim-planner/src/prepared.rs +++ b/rust-core/verisim-planner/src/prepared.rs @@ -755,6 +755,8 @@ mod tests { }, optimization_hint: Some("depth-limited BFS".to_string()), pushed_predicates: vec!["relates_to".to_string()], + projections: vec![], + early_limit: None, }], strategy: ExecutionStrategy::Sequential, total_cost: CostEstimate { diff --git a/rust-core/verisim-planner/src/profiler.rs b/rust-core/verisim-planner/src/profiler.rs index 36a2b726..eba6febd 100644 --- a/rust-core/verisim-planner/src/profiler.rs +++ b/rust-core/verisim-planner/src/profiler.rs @@ -463,6 +463,8 @@ mod tests { }, optimization_hint: Some("HNSW ANN search (k=10)".to_string()), pushed_predicates: vec!["Similarity { k: 10 }".to_string()], + projections: vec![], + early_limit: None, }, PlanStep { step: 2, @@ -475,6 +477,8 @@ mod tests { io_cost: 135.0, cpu_cost: 90.0, }, + projections: vec![], + early_limit: None, optimization_hint: Some("Graph traversal: relates_to (depth=2)".to_string()), pushed_predicates: vec!["Traversal { predicate: relates_to }".to_string()], }, diff --git a/rust-core/verisim-planner/src/slow_query.rs b/rust-core/verisim-planner/src/slow_query.rs index bcd29c55..290f3ea0 100644 --- a/rust-core/verisim-planner/src/slow_query.rs +++ b/rust-core/verisim-planner/src/slow_query.rs @@ -321,6 +321,8 @@ mod tests { }, optimization_hint: None, pushed_predicates: vec![], + projections: vec![], + early_limit: None, }) .collect(), strategy: if steps.len() >= 2 { From b93ff9c24f0e5116037ee3080bca8bb940baa88a Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:37:47 +0100 Subject: [PATCH 2/2] fix(planner): resolve the intra-doc link to CostModel::estimate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo doc` is a required check and `[`CostModel::estimate`]` does not resolve from `plan.rs` — the type lives in the sibling `cost` module and is not in scope there, so rustdoc failed the workspace build with "unresolved link". Qualified to `crate::cost::CostModel::estimate`. Verified with the same command CI runs (`cargo doc --no-deps`), plus `cargo test -p verisim-planner` still green. --- rust-core/verisim-planner/src/plan.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-core/verisim-planner/src/plan.rs b/rust-core/verisim-planner/src/plan.rs index 1d6ae8d3..1fff77ce 100644 --- a/rust-core/verisim-planner/src/plan.rs +++ b/rust-core/verisim-planner/src/plan.rs @@ -125,7 +125,7 @@ pub struct PlanStep { /// [`PlanNode::early_limit`]. /// /// Also absent until 2026-07-28, which was the more consequential half of - /// the same gap: [`CostModel::estimate`] *already* discounts a node + /// the same gap: [`crate::cost::CostModel::estimate`] *already* discounts a node /// carrying an early limit (it scales `selectivity` and takes `time_ms` /// down to as low as 50% of base), so a plan was scored cheaper for a /// pushdown the physical plan could not actually perform.