From c4e2b83031a246f34f4fb811c1bf7539b1d28e09 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 15:19:20 +0530 Subject: [PATCH 01/17] docs: add design spec for Wave 2 PR 1 (predicate push-down + mixed rule optimization) Covers issues #207 and #206: extends optimizer::plan() to accept Expr clauses and interleave them at the earliest position where their variables are bound, and routes the StratifiedEvaluator mixed-rules path through the updated planner. Co-Authored-By: Claude Sonnet 4.6 --- ...5-predicate-pushdown-mixed-rules-design.md | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md diff --git a/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md b/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md new file mode 100644 index 000000000..9fdbb1134 --- /dev/null +++ b/docs/superpowers/specs/2026-05-15-predicate-pushdown-mixed-rules-design.md @@ -0,0 +1,158 @@ +# Design: Predicate Push-down + Mixed Rule Optimization (Wave 2 PR 1) + +**Issues**: #207 (predicate push-down for expression clauses), #206 (optimize mixed rule evaluation) +**Date**: 2026-05-15 +**Branch**: single worktree + PR covering both issues + +--- + +## Summary + +Two related query execution improvements delivered in one PR: + +- **#207**: Push `Expr` predicate clauses (e.g. `[(> ?age 30)]`) down to the earliest position in the execution plan where all their variables are bound, rather than applying them all in a post-pass after full pattern matching. +- **#206**: Route the mixed-rules path in `StratifiedEvaluator` through the updated planner so it benefits from both pattern selectivity ordering and predicate push-down — no separate implementation needed. + +The unifying change is extending `optimizer::plan()` to accept both `Pattern` and `Expr` clauses and return an interleaved ordered plan. + +--- + +## Current Behaviour + +**Query execution (executor.rs)**: +``` +where_clauses + → extract Pattern variants + → plan(patterns) → sorted [(Pattern, IndexHint)] # selectivity order + → PatternMatcher runs all patterns → full binding set + → apply_expr_clauses(bindings) → post-pass filter # all exprs at the end +``` + +**Mixed rules (evaluator.rs `StratifiedEvaluator`)**: +``` +rule body + → extract Pattern variants only (in original order, no plan() call) + → PatternMatcher runs patterns → bindings + → not/notjoin post-filter +``` + +Both paths leave `Expr` clauses to the end, potentially carrying large intermediate binding sets through every pattern join before any filter is applied. + +--- + +## Design + +### 1. `plan()` — new signature and algorithm + +**File**: `src/query/datalog/optimizer.rs` + +```rust +// Before +pub fn plan( + patterns: Vec, + indexes: &Indexes, +) -> Vec<(Pattern, IndexHint)> + +// After +pub fn plan( + clauses: Vec, + indexes: &Indexes, +) -> Vec<(WhereClause, Option)> +``` + +Only `WhereClause::Pattern` and `WhereClause::Expr` variants are passed in. `Not`, `NotJoin`, `Or`, `OrJoin` continue to be extracted separately at call sites and applied as post-filters — their handling is unchanged. + +**Algorithm**: + +1. Separate input into patterns (assign `IndexHint` via `select_index`) and exprs. +2. `#[cfg(not(feature = "wasm"))]`: stable-sort patterns by selectivity descending — unchanged behaviour. +3. New helper `expr_vars(expr: &Expr) -> Vec` walks the `Expr` tree and collects referenced variable names (symbols starting with `?`). +4. Walk sorted patterns left-to-right, tracking the set of variables bound so far. For each `Expr`, insert it at the position where all its variables first become bound. +5. Exprs with no variables, or variables never bound by any pattern, are placed at the end. + +**`wasm` behaviour**: Pattern reordering remains gated behind `#[cfg(not(feature = "wasm"))]`. Expr push-down (step 4) is unconditional — it applies on all targets including WASM. + +**Return type**: `Vec<(WhereClause, Option)>` where `Pattern` entries carry `Some(IndexHint)` and `Expr` entries carry `None`. + +--- + +### 2. Executor inner loop + +**File**: `src/query/datalog/executor.rs` +**Functions**: `execute_query()`, `execute_query_with_rules()` + +Both functions currently use a two-phase structure: run all patterns, then call `apply_expr_clauses()`. After this change: + +1. Collect top-level `Pattern` and `Expr` clauses into a `Vec`. +2. Call `plan(clauses, indexes)` → interleaved `Vec<(WhereClause, Option)>`. +3. Process in a single ordered loop: + +``` +bindings = [empty_binding] +for (clause, hint) in planned: + WhereClause::Pattern(p) → + bindings = PatternMatcher::match_with_hint(p, hint, facts, bindings) + WhereClause::Expr { expr, binding: out } → + bindings = filter_or_extend(bindings, expr, out) +``` + +`Not`, `NotJoin`, `Or`, `OrJoin` are extracted before the plan call and applied as post-filters afterward — no change to their handling. The **top-level** `apply_expr_clauses()` post-pass (executor.rs lines ~811, ~1094) is removed; its logic moves inline into the ordered loop. `apply_expr_clauses()` calls inside `not`/`not-join` body evaluation (lines ~1070, ~1216) are **unchanged** — those Expr clauses belong to the not-body scope and are not candidates for push-down. + +--- + +### 3. Mixed-rules path + +**File**: `src/query/datalog/evaluator.rs` +**Function**: `StratifiedEvaluator::evaluate()` (mixed-rules loop, ~line 729) + +Currently collects `positive_patterns: Vec` in original body order, bypassing the planner entirely. + +After this change: +1. Collect `Pattern` and `Expr` `WhereClause` variants from the rule body. +2. Call `plan(clauses, indexes)` → interleaved plan. +3. Process with the same ordered loop as the executor. +4. `Not` and `NotJoin` filters continue to be applied as post-filters — no change. + +Mixed rules gain both pattern selectivity ordering and predicate push-down with no separate implementation, satisfying #206. + +--- + +### 4. Testing + +**`src/query/datalog/optimizer.rs` unit tests** — `plan()` with mixed input: +- Expr with all vars bound by first pattern → positioned immediately after that pattern +- Expr with vars spread across multiple patterns → positioned after the last binding pattern +- Expr with no variables → placed at end +- Expr referencing a variable no pattern binds → placed at end +- Under `wasm` feature: patterns stay in user-written order, Exprs still pushed down + +**`src/query/datalog/executor.rs` integration tests** — semantics preserved: +- Query with `[(> ?age 30)]` predicate produces same results before and after push-down +- Selective predicate on large fact set prunes intermediate bindings (verify via result count) + +**`src/query/datalog/evaluator.rs` unit tests** — mixed-rule path: +- Rule with `not` + `Expr` in body: Expr pushed down correctly, not-filter still applied afterward +- Rule with `or` + `Expr` in body: same + +**Benchmarks** (Criterion, per #207 acceptance criteria): +- `[(> ?age 30)]` selective predicate on large fact sets: throughput comparison before/after push-down + +--- + +## Invariants Preserved + +- **Correctness**: `eval_expr` returns `Err(())` for unbound variables (silently drops row). If push-down ever positions an Expr before all its vars are bound, the silent-drop behaviour acts as a safety net — results remain correct, just not optimally filtered. The `expr_vars()` helper must be correct to get the performance benefit. +- **Stratification safety**: `Not`/`NotJoin` handling is unchanged; push-down only applies to `Expr` clauses. +- **WASM portability**: Pattern reordering stays gated; Expr push-down is always on. +- **Backwards compatibility**: No change to query semantics, file format, or public API. + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `plan()` signature; new `expr_vars()` helper; push-down insertion algorithm | +| `src/query/datalog/executor.rs` | Updated call sites; single ordered processing loop; remove `apply_expr_clauses` post-pass | +| `src/query/datalog/evaluator.rs` | Mixed-rules path routes through `plan()` | +| `benches/` | New Criterion bench for selective predicate push-down | From b1c139d3637bb778cc9d406d902bb94284bf1c6d Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 15:37:36 +0530 Subject: [PATCH 02/17] docs: add Wave 2 PR 1 implementation plan (predicate push-down + mixed rules) TDD plan covering optimizer.rs, matcher.rs, executor.rs, evaluator.rs, and a new Criterion benchmark. 9 tasks, each with exact file paths, code blocks, and expected test output. Co-Authored-By: Claude Sonnet 4.6 --- ...26-05-15-predicate-pushdown-mixed-rules.md | 1459 +++++++++++++++++ 1 file changed, 1459 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md diff --git a/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md b/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md new file mode 100644 index 000000000..32b350f9d --- /dev/null +++ b/docs/superpowers/plans/2026-05-15-predicate-pushdown-mixed-rules.md @@ -0,0 +1,1459 @@ +# Wave 2 PR 1: Predicate Push-down + Mixed Rule Optimization + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extend `optimizer::plan()` to accept and interleave `Expr` predicate clauses at the earliest position where their variables are bound, and route the mixed-rules evaluator path through the updated planner. + +**Architecture:** `plan()` signature changes from `Vec → Vec<(Pattern, IndexHint)>` to `Vec → Vec<(WhereClause, Option)>`. The executor's inner join loop processes clauses in plan order (Pattern → join bindings, Expr → filter/extend inline). The `StratifiedEvaluator` mixed-rules path is updated to use the same planner. Three files modified: `optimizer.rs`, `executor.rs`, `evaluator.rs`. One new method added to `matcher.rs`. One new benchmark group. + +**Tech Stack:** Rust stable, `std::collections::HashSet` for variable tracking, Criterion for benchmarks. + +--- + +## File Map + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `expr_vars()`, `pattern_bound_vars()` helpers; `plan()` signature → `Vec`; update existing test | +| `src/query/datalog/matcher.rs` | New `pub(crate) match_with_hint_seeded()` method | +| `src/query/datalog/executor.rs` | Updated `execute_query()` and `execute_query_with_rules()`: inline ordered loop, registry acquired earlier, top-level `apply_expr_clauses` post-passes removed | +| `src/query/datalog/evaluator.rs` | `StratifiedEvaluator` mixed-rules loop routes through `plan()` | +| `benches/minigraf_bench.rs` | New `query/predicate_pushdown` benchmark group | + +--- + +## Task 1: Create git worktree + +**Files:** +- No file changes — shell commands only + +- [ ] **Step 1: Invoke the using-git-worktrees skill** + +``` +Use superpowers:using-git-worktrees to create a worktree for this feature. +Branch name: feature/issues-207-206-predicate-pushdown +Worktree dir: .worktrees/wave2-pr1-predicate-pushdown +``` + +- [ ] **Step 2: Verify worktree is active and clean** + +```bash +git worktree list +git status +``` + +Expected: worktree listed, status shows clean working tree on `feature/issues-207-206-predicate-pushdown`. + +--- + +## Task 2: Failing tests for `expr_vars()` and new `plan()` signature (optimizer.rs) + +**Files:** +- Modify: `src/query/datalog/optimizer.rs` (test module only) + +These tests compile-fail (new functions don't exist yet) and will pass after Task 3. + +- [ ] **Step 1: Add imports and test helpers to the test module** + +In `src/query/datalog/optimizer.rs`, extend the existing `#[cfg(test)] mod tests` block. Replace the current imports block: + +```rust +#[cfg(test)] +mod tests { + use super::*; + use crate::graph::types::Value; + use crate::query::datalog::types::{BinOp, EdnValue, Expr, Pattern, WhereClause}; + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + use uuid::Uuid; + + fn make_pattern(entity: EdnValue, attribute: EdnValue, value: EdnValue) -> Pattern { + Pattern::new(entity, attribute, value) + } + + fn var(s: &str) -> EdnValue { + EdnValue::Symbol(format!("?{s}")) + } + fn kw(s: &str) -> EdnValue { + EdnValue::Keyword(s.to_string()) + } + fn str_val(s: &str) -> EdnValue { + EdnValue::String(s.to_string()) + } + fn entity_lit() -> EdnValue { + EdnValue::Uuid(Uuid::new_v4()) + } +``` + +- [ ] **Step 2: Add failing tests for `expr_vars()`** + +Append to the test module: + +```rust + // ── expr_vars() ────────────────────────────────────────────────────────── + + #[test] + fn test_expr_vars_var() { + let e = Expr::Var("?age".to_string()); + assert_eq!(expr_vars(&e), vec!["?age".to_string()]); + } + + #[test] + fn test_expr_vars_lit_is_empty() { + let e = Expr::Lit(Value::Integer(42)); + assert!(expr_vars(&e).is_empty()); + } + + #[test] + fn test_expr_vars_binop() { + let e = Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?age".to_string())), + Box::new(Expr::Lit(Value::Integer(30))), + ); + assert_eq!(expr_vars(&e), vec!["?age".to_string()]); + } + + #[test] + fn test_expr_vars_nested_binop_collects_all() { + // (> (+ ?a ?b) ?c) + let e = Expr::BinOp( + BinOp::Gt, + Box::new(Expr::BinOp( + BinOp::Add, + Box::new(Expr::Var("?a".to_string())), + Box::new(Expr::Var("?b".to_string())), + )), + Box::new(Expr::Var("?c".to_string())), + ); + let vars = expr_vars(&e); + assert!(vars.contains(&"?a".to_string())); + assert!(vars.contains(&"?b".to_string())); + assert!(vars.contains(&"?c".to_string())); + assert_eq!(vars.len(), 3); + } + + #[test] + fn test_expr_vars_unary_op() { + use crate::query::datalog::types::UnaryOp; + let e = Expr::UnaryOp(UnaryOp::IntegerQ, Box::new(Expr::Var("?v".to_string()))); + assert_eq!(expr_vars(&e), vec!["?v".to_string()]); + } +``` + +- [ ] **Step 3: Add failing tests for new `plan()` return type and push-down** + +Append to the test module: + +```rust + // ── plan() — new signature and push-down ───────────────────────────────── + + #[test] + fn test_plan_pattern_carries_some_hint() { + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + #[cfg(not(feature = "wasm"))] + { + let p = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let planned = plan(vec![p], &Indexes::new()); + assert!(planned[0].1.is_some(), "Pattern entry must carry Some(IndexHint)"); + } + } + + #[test] + fn test_plan_expr_carries_none_hint() { + #[cfg(not(feature = "wasm"))] + use crate::storage::index::Indexes; + #[cfg(not(feature = "wasm"))] + { + let p = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::Lit(Value::Boolean(true)), + binding: None, + }; + let planned = plan(vec![p, expr], &Indexes::new()); + let expr_entry = planned.iter().find(|(c, _)| matches!(c, WhereClause::Expr { .. })); + assert!(expr_entry.is_some()); + assert!(expr_entry.unwrap().1.is_none(), "Expr entry must carry None hint"); + } + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_pushed_after_binding_pattern() { + // Three patterns with equal selectivity (1 attr bound each) — stable sort preserves + // original order: [p1, p2, p3]. Expr needs ?v, bound by p2 (pos 1). + // Expected output: [p1, p2, expr, p3]. + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":name"), var("n"))); + let p2 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let p3 = WhereClause::Pattern(make_pattern(var("e"), kw(":dept"), var("d"))); + let expr = WhereClause::Expr { + expr: Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?v".to_string())), + Box::new(Expr::Lit(Value::Integer(30))), + ), + binding: None, + }; + let planned = plan(vec![p1, p2, p3, expr], &Indexes::new()); + assert_eq!(planned.len(), 4); + // Item at index 2 must be the Expr (pushed after p2 which binds ?v at index 1). + assert!( + matches!(planned[2].0, WhereClause::Expr { .. }), + "Expr must be at index 2" + ); + // Item at index 3 must be a Pattern (p3, not yet seen when ?v first bound). + assert!( + matches!(planned[3].0, WhereClause::Pattern(_)), + "p3 must be at index 3" + ); + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_no_vars_goes_to_end() { + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::Lit(Value::Boolean(true)), + binding: None, + }; + let planned = plan(vec![p1, expr], &Indexes::new()); + assert_eq!(planned.len(), 2); + assert!( + matches!(planned[1].0, WhereClause::Expr { .. }), + "no-var Expr must be last" + ); + } + + #[cfg(not(feature = "wasm"))] + #[test] + fn test_expr_unbound_var_goes_to_end() { + // ?x is never bound by any pattern + let p1 = WhereClause::Pattern(make_pattern(var("e"), kw(":val"), var("v"))); + let expr = WhereClause::Expr { + expr: Expr::BinOp( + BinOp::Gt, + Box::new(Expr::Var("?x".to_string())), + Box::new(Expr::Lit(Value::Integer(0))), + ), + binding: None, + }; + let planned = plan(vec![p1, expr], &Indexes::new()); + assert_eq!(planned.len(), 2); + assert!( + matches!(planned[1].0, WhereClause::Expr { .. }), + "Expr with unbound var must be last" + ); + } +``` + +- [ ] **Step 4: Run to verify tests fail to compile** + +```bash +cargo test --lib -p minigraf -- query::datalog::optimizer 2>&1 | head -30 +``` + +Expected: compile errors — `expr_vars`, `pattern_bound_vars` not found; `plan()` type mismatch. + +--- + +## Task 3: Implement `expr_vars()`, `pattern_bound_vars()`, and new `plan()` (optimizer.rs) + +**Files:** +- Modify: `src/query/datalog/optimizer.rs` + +- [ ] **Step 1: Update imports at the top of optimizer.rs** + +Replace the existing import line: + +```rust +use crate::query::datalog::types::{AttributeSpec, EdnValue, Pattern}; +``` + +with: + +```rust +use crate::query::datalog::types::{AttributeSpec, EdnValue, Expr, Pattern, WhereClause}; +``` + +- [ ] **Step 2: Add `expr_vars()` and `pattern_bound_vars()` helpers after the existing helpers** + +Add after the `attr_is_index_bound` function (around line 38): + +```rust +/// Collect all logic-variable names (`?foo`) referenced in an Expr tree. +fn expr_vars(expr: &Expr) -> Vec { + match expr { + Expr::Var(s) => vec![s.clone()], + Expr::Lit(_) | Expr::Slot(_) => vec![], + Expr::BinOp(_, l, r) => { + let mut vars = expr_vars(l); + vars.extend(expr_vars(r)); + vars + } + Expr::UnaryOp(_, inner) => expr_vars(inner), + } +} + +/// Collect the logic-variable names bound (output) by a Pattern. +/// Only Symbol values starting with `?` count — literals never bind. +fn pattern_bound_vars(p: &Pattern) -> Vec { + let mut vars = Vec::new(); + if is_variable(&p.entity) { + if let EdnValue::Symbol(s) = &p.entity { + vars.push(s.clone()); + } + } + if let AttributeSpec::Real(attr) = &p.attribute { + if is_variable(attr) { + if let EdnValue::Symbol(s) = attr { + vars.push(s.clone()); + } + } + } + if is_variable(&p.value) { + if let EdnValue::Symbol(s) = &p.value { + vars.push(s.clone()); + } + } + vars +} +``` + +- [ ] **Step 3: Replace `plan()` with the new signature and push-down algorithm** + +Replace the entire `plan()` function (currently lines 86–108): + +```rust +/// Plan a list of where clauses: assign index hints to Pattern entries, push Expr +/// entries to the earliest position where all their variables are bound by preceding +/// patterns, and (non-wasm) sort patterns by selectivity. +/// +/// Only `WhereClause::Pattern` and `WhereClause::Expr` variants should be passed in. +/// `Not`, `NotJoin`, `Or`, `OrJoin`, and `RuleInvocation` variants are handled by +/// the executor/evaluator and must not appear here. +/// +/// Returns an interleaved `Vec<(WhereClause, Option)>` where Pattern entries +/// carry `Some(hint)` and Expr entries carry `None`. +pub fn plan( + clauses: Vec, + _indexes: &crate::storage::index::Indexes, +) -> Vec<(WhereClause, Option)> { + // Separate into patterns (with hints) and exprs. + let mut patterns: Vec<(WhereClause, IndexHint)> = Vec::new(); + let mut exprs: Vec = Vec::new(); + + for clause in clauses { + match &clause { + WhereClause::Pattern(p) => { + let hint = select_index(p); + patterns.push((clause, hint)); + } + WhereClause::Expr { .. } => exprs.push(clause), + // Other variants must not be passed to plan(); silently skip. + _ => {} + } + } + + // Stable sort patterns by selectivity descending (non-wasm only). + // Preserves original order for ties, ensuring deterministic output. + #[cfg(not(feature = "wasm"))] + patterns.sort_by_key(|(clause, _)| { + if let WhereClause::Pattern(p) = clause { + std::cmp::Reverse(selectivity_score(p)) + } else { + std::cmp::Reverse(0u8) + } + }); + + // Start with sorted patterns only. + let mut result: Vec<(WhereClause, Option)> = patterns + .into_iter() + .map(|(clause, hint)| (clause, Some(hint))) + .collect(); + + // Push each Expr to the earliest position where all its variables are bound. + for expr_clause in exprs { + let vars: std::collections::HashSet = + if let WhereClause::Expr { expr, .. } = &expr_clause { + expr_vars(expr).into_iter().collect() + } else { + Default::default() + }; + + let mut bound: std::collections::HashSet = Default::default(); + // Default: append at end (covers no-var Exprs and vars never bound by any pattern). + let mut insert_pos = result.len(); + + if !vars.is_empty() { + for (pos, (clause, _)) in result.iter().enumerate() { + if let WhereClause::Pattern(p) = clause { + bound.extend(pattern_bound_vars(p)); + if vars.is_subset(&bound) { + insert_pos = pos + 1; + break; + } + } + } + } + + result.insert(insert_pos, (expr_clause, None)); + } + + result +} +``` + +- [ ] **Step 4: Update the existing join-ordering test to use the new signature** + +Find and replace `test_join_ordering_moves_selective_pattern_first` in the test module: + +```rust + #[cfg(not(feature = "wasm"))] + #[test] + fn test_join_ordering_moves_selective_pattern_first() { + let p1 = make_pattern(var("e"), kw(":age"), var("a")); // selectivity 1 (attr only) + let p2 = make_pattern(entity_lit(), kw(":name"), var("v")); // selectivity 2 (entity + attr) + let p1_attr = p1.attribute.clone(); + let p2_attr = p2.attribute.clone(); + let planned = plan( + vec![ + WhereClause::Pattern(p1), + WhereClause::Pattern(p2), + ], + &Indexes::new(), + ); + // planned[0].0 is WhereClause::Pattern — extract the inner Pattern. + let first_attr = match &planned[0].0 { + WhereClause::Pattern(p) => p.attribute.clone(), + _ => panic!("expected Pattern at index 0"), + }; + let second_attr = match &planned[1].0 { + WhereClause::Pattern(p) => p.attribute.clone(), + _ => panic!("expected Pattern at index 1"), + }; + assert_ne!(first_attr, p1_attr, "Lower-selectivity pattern must not be first"); + assert_eq!(first_attr, p2_attr, "Higher-selectivity pattern must be first"); + assert_eq!(second_attr, p1_attr, "Lower-selectivity pattern must be second"); + } +``` + +- [ ] **Step 5: Run optimizer tests to verify they pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::optimizer 2>&1 +``` + +Expected: all optimizer tests pass, no compile errors. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/optimizer.rs +git commit -m "feat(optimizer): extend plan() to accept Expr clauses with push-down positioning + +Adds expr_vars() and pattern_bound_vars() helpers. plan() now accepts +Vec (Pattern + Expr) and returns Vec<(WhereClause, Option)>. +Each Expr is inserted at the earliest position after all its variables are +bound by preceding patterns. No-variable Exprs and Exprs with unbound +variables are appended at the end. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 4: Add `match_with_hint_seeded()` to PatternMatcher (matcher.rs) + +**Files:** +- Modify: `src/query/datalog/matcher.rs` + +- [ ] **Step 1: Write a failing test for `match_with_hint_seeded()`** + +Add to the test module in `src/query/datalog/matcher.rs`: + +```rust + #[test] + fn test_match_with_hint_seeded_unit_seed_uses_hint_path() { + // Unit seed (one empty map) — behaves like the first pattern in a query. + // Any non-empty result confirms the method returns matches (not empty). + use crate::query::datalog::optimizer::IndexHint; + let storage = FactStorage::new(); + storage.load_fact(crate::graph::types::Fact { + entity: uuid::Uuid::new_v4(), + attribute: ":val".to_string(), + value: crate::graph::types::Value::Integer(1), + tx_id: 1, + tx_count: 1, + valid_from: 0, + valid_to: i64::MAX, + asserted: true, + }).unwrap(); + let facts: Arc<[crate::graph::types::Fact]> = + Arc::from(storage.get_asserted_facts().unwrap()); + let matcher = PatternMatcher::from_slice(facts); + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":val".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + let unit_seed = vec![HashMap::new()]; + let results = matcher.match_with_hint_seeded(unit_seed, &p, &IndexHint::Aevt); + assert_eq!(results.len(), 1, "unit seed must produce one result per matching fact"); + } + + #[test] + fn test_match_with_hint_seeded_real_bindings_uses_join() { + // Non-unit seed — behaves like a subsequent pattern (join path). + use crate::query::datalog::optimizer::IndexHint; + use uuid::Uuid; + let storage = FactStorage::new(); + let e = Uuid::new_v4(); + storage.load_fact(crate::graph::types::Fact { + entity: e, + attribute: ":val".to_string(), + value: crate::graph::types::Value::Integer(42), + tx_id: 1, + tx_count: 1, + valid_from: 0, + valid_to: i64::MAX, + asserted: true, + }).unwrap(); + let facts: Arc<[crate::graph::types::Fact]> = + Arc::from(storage.get_asserted_facts().unwrap()); + let matcher = PatternMatcher::from_slice(facts); + let p = Pattern::new( + EdnValue::Uuid(e), + EdnValue::Keyword(":val".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + // Seed with a binding that already has some data (non-unit). + let seed = vec![{ + let mut m = HashMap::new(); + m.insert("?other".to_string(), crate::graph::types::Value::Integer(99)); + m + }]; + let results = matcher.match_with_hint_seeded(seed, &p, &IndexHint::Eavt); + assert_eq!(results.len(), 1, "join path must unify with existing binding"); + assert_eq!( + results[0].get("?v"), + Some(&crate::graph::types::Value::Integer(42)) + ); + } +``` + +- [ ] **Step 2: Run to confirm tests fail to compile** + +```bash +cargo test --lib -p minigraf -- query::datalog::matcher 2>&1 | head -20 +``` + +Expected: compile error — `match_with_hint_seeded` not found. + +- [ ] **Step 3: Implement `match_with_hint_seeded()`** + +Add the following method to the `impl PatternMatcher` block in `src/query/datalog/matcher.rs`, after `match_patterns_with_hints`: + +```rust + /// Match a single pattern against existing bindings, using an index hint when the + /// bindings are the unit seed (one empty map — i.e., this is the first pattern). + /// + /// When `seed` is the unit binding (`[{}]`), delegates to `match_pattern_with_hint` + /// for an indexed lookup. For all other seeds, delegates to `join_with_pattern` + /// (hash-join path) which already picks the right strategy. + /// + /// Used by the executor and evaluator incremental plan loops introduced in #207. + pub(crate) fn match_with_hint_seeded( + &self, + seed: Vec, + pattern: &Pattern, + hint: &crate::query::datalog::optimizer::IndexHint, + ) -> Vec { + if seed.len() == 1 && seed[0].is_empty() { + // First pattern in the plan — use the index hint for a targeted lookup. + self.match_pattern_with_hint(pattern, hint) + } else { + // Subsequent pattern — join_with_pattern uses hash-join when possible. + self.join_with_pattern(seed, pattern) + } + } +``` + +- [ ] **Step 4: Run matcher tests to confirm they pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::matcher 2>&1 +``` + +Expected: all matcher tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/matcher.rs +git commit -m "feat(matcher): add match_with_hint_seeded() for incremental plan execution + +Used by the executor and evaluator plan loops added in #207: applies +an index hint for the first pattern (unit seed) and falls back to the +hash-join path for subsequent patterns. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 5: Update `execute_query()` in executor.rs + +**Files:** +- Modify: `src/query/datalog/executor.rs` + +- [ ] **Step 1: Write integration tests that assert semantics are preserved** + +Add to the `#[cfg(test)]` module in `src/query/datalog/executor.rs`: + +```rust + #[test] + fn test_expr_pushdown_preserves_query_results() { + // Query with a selective Expr predicate: results must be identical before and after + // the push-down refactor. This test serves as a regression guard. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 10] [:e2 :val 20] [:e3 :val 30]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e ?v :where [?e :val ?v] [(> ?v 15)]])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "only :e2 and :e3 have :val > 15"); + } else { + panic!("expected QueryResults"); + } + } + + #[test] + fn test_expr_pushdown_multi_pattern_preserves_results() { + // Multi-pattern query with Expr in the middle: [?e :val ?v] [?e :name ?n] [(> ?v 10)] + // Expr should be pushed between the two patterns. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute(r#"(transact [[:e1 :val 5] [:e1 :name "a"] [:e2 :val 20] [:e2 :name "b"]])"#) + .unwrap(); + let result = executor + .execute(r#"(query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v 10)]])"#) + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 1, "only :e2 passes the predicate"); + } else { + panic!("expected QueryResults"); + } + } + + #[test] + fn test_expr_binding_form_preserves_results() { + // Binding-form Expr: [(* ?v 2) ?doubled] — should still work after refactor. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 5] [:e2 :val 10]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e ?doubled :where [?e :val ?v] [(* ?v 2) ?doubled]])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "both entities must appear"); + } else { + panic!("expected QueryResults"); + } + } +``` + +- [ ] **Step 2: Run to confirm tests pass against the current (pre-refactor) code** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor::tests::test_expr_pushdown 2>&1 +``` + +Expected: all three new tests PASS (they test semantics, not the push-down path). + +- [ ] **Step 3: Refactor `execute_query()` — move registry acquisition and replace pattern/expr handling** + +In `execute_query()`, make the following changes: + +**3a.** Move registry acquisition to BEFORE the pattern matching block. Replace the current sequence (starting around line 493): + +```rust + let patterns = query.get_patterns(); + + // Plan patterns: assign index hints and reorder by selectivity. + let planned_patterns = optimizer::plan(patterns, &self.indexes); + + // Match all patterns in planned order and get bindings + let bindings = matcher.match_patterns_with_hints(&planned_patterns); + + // Acquire the function registry once; used by apply_or_clauses, not_body_matches, + // apply_expr_clauses, and apply_post_processing below. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; +``` + +with: + +```rust + // Acquire function registry before the plan loop — needed for inline Expr evaluation. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; + + // Pre-validate UDF predicate names: surface unknown predicates as errors before + // processing any rows (matches the behaviour of the former apply_expr_clauses post-pass). + for clause in &query.where_clauses { + if let WhereClause::Expr { + expr: Expr::UnaryOp(UnaryOp::Udf(name), _), + .. + } = clause + { + if registry.get_predicate(name).is_none() { + anyhow::bail!("unknown predicate: '{}'", name); + } + } + } + + // Collect Pattern and Expr top-level clauses for the planner. + // Not/NotJoin/Or/OrJoin are extracted separately below and applied as post-filters. + let plan_clauses: Vec = query + .where_clauses + .iter() + .filter(|c| matches!(c, WhereClause::Pattern(_) | WhereClause::Expr { .. })) + .cloned() + .collect(); + + let planned = optimizer::plan(plan_clauses, &self.indexes); + + // Process planned clauses in order: Pattern → expand bindings, Expr → filter/extend. + let mut bindings: Vec> = + vec![std::collections::HashMap::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + bindings = matcher.match_with_hint_seeded( + bindings, + &p, + hint.as_ref().unwrap_or(&optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + bindings = bindings + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(®istry)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } +``` + +**3b.** Remove the top-level `apply_expr_clauses` post-pass. Find and delete (around line 810): + +```rust + // Apply WhereClause::Expr clauses (filter and binding predicates) + let filtered_bindings = apply_expr_clauses(not_filtered, &query.where_clauses, ®istry)?; +``` + +Replace the reference to `filtered_bindings` in the line immediately after with `not_filtered`: + +```rust + let results = + apply_post_processing(not_filtered, &query.find, &query.with_vars, ®istry)?; +``` + +- [ ] **Step 4: Run tests to confirm they still pass** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor 2>&1 +``` + +Expected: all executor tests pass, including the three new ones. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): inline Expr push-down in execute_query() plan loop + +Replaces the two-phase pattern-match-then-expr-filter with a single +ordered loop that processes Pattern (join) and Expr (filter/extend) +clauses in plan order. Registry is acquired before the loop for inline +Expr evaluation. Removes the top-level apply_expr_clauses post-pass. +Not/NotJoin/Or/OrJoin handling is unchanged. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 6: Update `execute_query_with_rules()` in executor.rs + +**Files:** +- Modify: `src/query/datalog/executor.rs` + +- [ ] **Step 1: Write integration tests for the rules path** + +Add to the test module in `src/query/datalog/executor.rs`: + +```rust + #[test] + fn test_expr_pushdown_with_rules_preserves_results() { + // Query that uses rules AND an Expr predicate — exercises execute_query_with_rules. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage.clone()); + executor + .execute("(transact [[:e1 :val 5] [:e2 :val 20] [:e3 :val 30]])") + .unwrap(); + executor + .execute("(rule [(high ?e) [?e :val ?v] [(> ?v 15)]])") + .unwrap(); + let result = executor + .execute("(query [:find ?e :where (high ?e)])") + .unwrap(); + if let QueryResult::QueryResults { results, .. } = result { + assert_eq!(results.len(), 2, "only :e2 and :e3 qualify"); + } else { + panic!("expected QueryResults"); + } + } +``` + +- [ ] **Step 2: Run to confirm test passes against current code** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor::tests::test_expr_pushdown_with_rules 2>&1 +``` + +Expected: PASS (semantics test; push-down not yet applied here). + +- [ ] **Step 3: Refactor `execute_query_with_rules()` — replace pattern/expr handling** + +In `execute_query_with_rules()`, replace the current sequence (around lines 881–924): + +```rust + // Convert ONLY top-level rule invocations to positive-match patterns. + // Rule invocations inside `not` bodies are handled by the not-post-filter below. + // (reachable ?x ?y) becomes [?x :reachable ?y] + let mut all_patterns = query.get_patterns(); + + for (predicate, args) in query.get_top_level_rule_invocations() { + let pattern = match args.len() { + 1 => { ... } + 2 => { ... } + n => { return Err(...); } + }; + all_patterns.push(pattern); + } + + // Compute derived_facts Arc once; reuse for or-clauses and not-post-filter. + let derived_facts: Arc<[Fact]> = ...; + + // Match all patterns against derived facts + let matcher = PatternMatcher::from_slice_with_valid_at(derived_facts.clone(), valid_at_value.clone()); + let bindings = matcher.match_patterns(&all_patterns); + + // Acquire the function registry once; ... + let registry = self.functions.read()...; +``` + +with: + +```rust + // Compute derived_facts Arc once; reuse for plan loop, or-clauses and not-post-filter. + // Must use derived_storage (includes rule-derived facts), not filtered_facts (base only). + let derived_facts: Arc<[Fact]> = + Arc::from(derived_storage.get_asserted_facts().unwrap_or_default()); + + let matcher = + PatternMatcher::from_slice_with_valid_at(derived_facts.clone(), valid_at_value.clone()); + + // Acquire function registry before the plan loop — needed for inline Expr evaluation. + let registry = self + .functions + .read() + .map_err(|_| anyhow!("functions lock poisoned"))?; + + // Pre-validate UDF predicate names. + for clause in &query.where_clauses { + if let WhereClause::Expr { + expr: Expr::UnaryOp(UnaryOp::Udf(name), _), + .. + } = clause + { + if registry.get_predicate(name).is_none() { + anyhow::bail!("unknown predicate: '{}'", name); + } + } + } + + // Collect Pattern and Expr top-level clauses for the planner. + // Rule invocations are converted to WhereClause::Pattern against derived_storage. + let mut plan_clauses: Vec = query + .where_clauses + .iter() + .filter(|c| matches!(c, WhereClause::Pattern(_) | WhereClause::Expr { .. })) + .cloned() + .collect(); + + for (predicate, args) in query.get_top_level_rule_invocations() { + let pattern = match args.len() { + 1 => { + #[allow(clippy::indexing_slicing)] + let entity = args[0].clone(); + Pattern::new( + entity, + EdnValue::Keyword(format!(":{}", predicate)), + EdnValue::Symbol("?_rule_value".to_string()), + ) + } + 2 => { + #[allow(clippy::indexing_slicing)] + let entity = args[0].clone(); + #[allow(clippy::indexing_slicing)] + let value = args[1].clone(); + Pattern::new(entity, EdnValue::Keyword(format!(":{}", predicate)), value) + } + n => { + return Err(anyhow!( + "Rule invocation '{}' must have 1 or 2 arguments, got {}", + predicate, + n + )); + } + }; + plan_clauses.push(WhereClause::Pattern(pattern)); + } + + let planned = optimizer::plan(plan_clauses, &self.indexes); + + // Process planned clauses in order: Pattern → expand, Expr → filter/extend. + let mut bindings: Vec> = + vec![std::collections::HashMap::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + bindings = matcher.match_with_hint_seeded( + bindings, + &p, + hint.as_ref().unwrap_or(&optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + bindings = bindings + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(®istry)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } +``` + +**3b.** Remove the top-level `apply_expr_clauses` post-pass in this function. Find and delete (around line 1094): + +```rust + // Apply WhereClause::Expr clauses (filter and binding predicates) + let filtered_bindings = apply_expr_clauses(not_filtered, &query.where_clauses, ®istry)?; +``` + +Replace the `filtered_bindings` reference immediately after with `not_filtered`: + +```rust + let results = + apply_post_processing(not_filtered, &query.find, &query.with_vars, ®istry)?; +``` + +- [ ] **Step 4: Run all executor tests** + +```bash +cargo test --lib -p minigraf -- query::datalog::executor 2>&1 +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/query/datalog/executor.rs +git commit -m "feat(executor): inline Expr push-down in execute_query_with_rules() plan loop + +Mirrors Task 5 for the rule-backed query path. Rule invocations are +converted to WhereClause::Pattern before the plan call so they +participate in selectivity ordering. Removes second apply_expr_clauses +post-pass. + +Closes part of #207. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 7: Update evaluator mixed-rules path (evaluator.rs) — closes #206 + +**Files:** +- Modify: `src/query/datalog/evaluator.rs` + +- [ ] **Step 1: Write integration test for mixed-rules Expr push-down** + +Add to the test module in `src/query/datalog/evaluator.rs`: + +```rust + #[test] + fn test_mixed_rule_with_expr_preserves_semantics() { + // Rule with not + Expr: (eligible ?x) :- [?x :val ?v] [(> ?v 10)] (not [?x :blocked true]) + // Expr should be pushed after [?x :val ?v] in the mixed-rules plan. + // Result must be the same as before the refactor. + let storage = FactStorage::new(); + let rules = Arc::new(RwLock::new(RuleRegistry::new())); + let functions = Arc::new(RwLock::new(FunctionRegistry::with_builtins())); + + let e1 = Uuid::new_v4(); + let e2 = Uuid::new_v4(); + let e3 = Uuid::new_v4(); + + storage + .transact( + vec![ + (e1, ":val".to_string(), Value::Integer(5)), + (e2, ":val".to_string(), Value::Integer(20)), + (e3, ":val".to_string(), Value::Integer(30)), + (e3, ":blocked".to_string(), Value::Boolean(true)), + ], + None, + ) + .unwrap(); + + register_test_rule( + &rules, + r#"(rule [(eligible ?x) [?x :val ?v] [(> ?v 10)] (not [?x :blocked true])])"#, + ); + + let evaluator = StratifiedEvaluator::new( + storage, + Arc::clone(&rules), + Arc::clone(&functions), + DEFAULT_MAX_ITERATIONS, + DEFAULT_MAX_DERIVED_FACTS, + DEFAULT_MAX_RESULTS, + ); + + let derived = evaluator.evaluate(&["eligible".to_string()]).unwrap(); + let facts = derived.get_asserted_facts().unwrap(); + let eligible: Vec<_> = facts + .iter() + .filter(|f| f.attribute.contains("eligible")) + .collect(); + assert_eq!(eligible.len(), 1, "only e2 is eligible (e3 is blocked)"); + } +``` + +- [ ] **Step 2: Run to confirm test passes against current code** + +```bash +cargo test --lib -p minigraf -- query::datalog::evaluator::tests::test_mixed_rule_with_expr 2>&1 +``` + +Expected: PASS (semantics test). + +- [ ] **Step 3: Refactor the mixed-rules loop in `StratifiedEvaluator::evaluate()`** + +In `src/query/datalog/evaluator.rs`, inside the mixed-rules loop (around line 729), replace the `positive_patterns` + `body_expr_clauses` extraction and the `raw_candidates` + `candidates` computation: + +**Current code to replace** (roughly lines 729–827): + +```rust + let positive_patterns: Vec = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::Pattern(p) => Some(p.clone()), + WhereClause::RuleInvocation { predicate, args } => { ... } + WhereClause::Not(_) | WhereClause::NotJoin { .. } => None, + WhereClause::Expr { .. } => None, + WhereClause::Or(_) | WhereClause::OrJoin { .. } => None, + }) + .collect(); + // ... not_clauses / not_join_clauses extraction ... + let body_expr_clauses: Vec<&WhereClause> = rule.body.iter() + .filter(|c| matches!(c, WhereClause::Expr { .. })).collect(); + let accumulated_facts: Arc<[Fact]> = ...; + let matcher = PatternMatcher::from_slice(accumulated_facts.clone()); + let raw_candidates = matcher.match_patterns(&positive_patterns); + // apply_or_clauses → or_expanded + let candidates = + apply_expr_clauses_in_evaluator(or_expanded, &body_expr_clauses, &fn_guard); +``` + +**New code**: + +```rust + // Collect Pattern and Expr clauses for the planner. + // RuleInvocations are converted to Pattern; Not/NotJoin/Or/OrJoin extracted separately below. + let mut plan_clauses: Vec = Vec::new(); + for c in &rule.body { + match c { + WhereClause::Pattern(_) | WhereClause::Expr { .. } => { + plan_clauses.push(c.clone()); + } + WhereClause::RuleInvocation { predicate, args } => { + let pattern = match args.len() { + 1 => args.first().map(|a0| { + Pattern::new( + a0.clone(), + EdnValue::Keyword(format!(":{}", predicate)), + EdnValue::Symbol("?_rule_value".to_string()), + ) + }), + 2 => args.first().and_then(|a0| { + args.get(1).map(|a1| { + Pattern::new( + a0.clone(), + EdnValue::Keyword(format!(":{}", predicate)), + a1.clone(), + ) + }) + }), + _ => None, + }; + if let Some(p) = pattern { + plan_clauses.push(WhereClause::Pattern(p)); + } + } + WhereClause::Not(_) + | WhereClause::NotJoin { .. } + | WhereClause::Or(_) + | WhereClause::OrJoin { .. } => {} + } + } + + let not_clauses: Vec> = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::Not(inner) => Some(inner.clone()), + _ => None, + }) + .collect(); + + let not_join_clauses: Vec<(Vec, Vec)> = rule + .body + .iter() + .filter_map(|c| match c { + WhereClause::NotJoin { join_vars, clauses } => { + Some((join_vars.clone(), clauses.clone())) + } + _ => None, + }) + .collect(); + + // Compute once; reuse for plan loop, apply_or_clauses, not-body matching. + let accumulated_facts: Arc<[Fact]> = + Arc::from(accumulated.get_asserted_facts().unwrap_or_default()); + + let matcher = PatternMatcher::from_slice(accumulated_facts.clone()); + + // Plan: assigns index hints + pushes Expr to earliest binding position. + let planned = crate::query::datalog::optimizer::plan( + plan_clauses, + &crate::storage::index::Indexes::new(), + ); + + // Process planned clauses in order: Pattern → join, Expr → filter/extend. + let fn_guard = self + .functions + .read() + .map_err(|_| anyhow!("function registry lock poisoned"))?; + let mut candidates: Vec = vec![Bindings::new()]; + for (clause, hint) in planned { + match clause { + WhereClause::Pattern(p) => { + candidates = matcher.match_with_hint_seeded( + candidates, + &p, + hint.as_ref() + .unwrap_or(&crate::query::datalog::optimizer::IndexHint::Eavt), + ); + } + WhereClause::Expr { expr, binding: out } => { + use crate::query::datalog::executor::{eval_expr, is_truthy}; + candidates = candidates + .into_iter() + .filter_map(|mut b| { + match eval_expr(&expr, &b, Some(&fn_guard)) { + Ok(v) => { + if let Some(var) = &out { + b.insert(var.clone(), v); + Some(b) + } else if is_truthy(&v) { + Some(b) + } else { + None + } + } + Err(_) => None, + } + }) + .collect(); + } + _ => {} + } + } + drop(fn_guard); + + // Apply Or/OrJoin clauses (mirrors top-level execute_query order). + let candidates = { + use crate::query::datalog::executor::apply_or_clauses; + let registry_guard = self + .rules + .read() + .map_err(|_| anyhow!("rule registry lock poisoned"))?; + let fn_registry = crate::query::datalog::functions::FunctionRegistry::with_builtins(); + let expanded = apply_or_clauses( + &rule.body, + candidates, + accumulated_facts.clone(), + ®istry_guard, + None, + None, + &fn_registry, + )?; + drop(registry_guard); + expanded + }; +``` + +The remaining `'binding: for binding in candidates { ... }` loop (not-body filtering + head instantiation) is **unchanged**. + +- [ ] **Step 4: Run evaluator tests** + +```bash +cargo test --lib -p minigraf -- query::datalog::evaluator 2>&1 +``` + +Expected: all tests pass. + +- [ ] **Step 5: Run all library tests** + +```bash +cargo test --lib -p minigraf 2>&1 +``` + +Expected: all tests pass. Fix any compile errors from call-site changes. + +- [ ] **Step 6: Commit** + +```bash +git add src/query/datalog/evaluator.rs +git commit -m "feat(evaluator): route mixed-rules path through plan() for Expr push-down + +StratifiedEvaluator mixed-rules loop now collects Pattern + Expr clauses +and calls optimizer::plan(), processing them in the same ordered loop as +the executor. Expr clauses are pushed down to the earliest valid position +in the rule body. Not/NotJoin filtering is unchanged. + +Closes #206. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 8: Add predicate push-down benchmark + +**Files:** +- Modify: `benches/minigraf_bench.rs` +- Modify: `benches/helpers/mod.rs` (optional — `populate_with_names` already exists) + +- [ ] **Step 1: Add a new `bench_predicate_pushdown` function in `benches/minigraf_bench.rs`** + +Add after the last existing bench function, before `criterion_group!`: + +```rust +// ── Task 8: query/predicate_pushdown ───────────────────────────────────────── + +fn bench_predicate_pushdown(c: &mut Criterion) { + // Fixture: n entities each with :val (integer) and :name (string). + // Uses helpers::populate_with_names(n). + // + // Query under test: multi-pattern with a selective Expr predicate. + // (query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v )]]) + // + // Predicate selects 10% of entities. With push-down, [?e :name ?n] is only + // joined for the 10% that pass the filter; without push-down it would join all. + const SCALES: &[(&str, usize)] = &[ + ("1k", 1_000), + ("10k", 10_000), + ("100k", 100_000), + ]; + + let mut group = c.benchmark_group("query/predicate_pushdown"); + group.sample_size(10); + + for &(label, n) in SCALES { + let db = helpers::populate_with_names(n); + // Threshold = 90th percentile value → 10% of entities pass the predicate. + let threshold = (n as i64) * 9 / 10; + let query = format!( + "(query [:find ?e ?n :where [?e :val ?v] [?e :name ?n] [(> ?v {})]])", + threshold + ); + group.bench_with_input( + BenchmarkId::from_parameter(label), + &(db, query), + |b, (db, q)| { + b.iter(|| db.execute(q).unwrap()); + }, + ); + } + + group.finish(); +} +``` + +- [ ] **Step 2: Register the benchmark in `criterion_group!`** + +Find the `criterion_group!` macro and add `bench_predicate_pushdown`: + +```rust +criterion_group!( + benches, + bench_insert, + bench_insert_file, + // ... existing entries ... + bench_predicate_pushdown, +); +``` + +- [ ] **Step 3: Verify the benchmark compiles and runs** + +```bash +cargo bench --bench minigraf_bench -- query/predicate_pushdown 2>&1 | tail -20 +``` + +Expected: benchmark runs and prints throughput numbers, no panics. + +- [ ] **Step 4: Commit** + +```bash +git add benches/minigraf_bench.rs +git commit -m "bench: add query/predicate_pushdown benchmark group + +Measures multi-pattern query with a selective [(> ?v N)] predicate at +1K/10K/100K fact scales. Threshold set at 90th percentile (10% pass rate). +Provides baseline for evaluating push-down gains. + +Co-Authored-By: Claude Sonnet 4.6 " +``` + +--- + +## Task 9: Full test suite, final cleanup, open PR + +**Files:** +- No new file changes — verification and PR creation + +- [ ] **Step 1: Run the full test suite** + +```bash +cargo test 2>&1 +``` + +Expected: all tests pass (unit + integration + doc). Fix any remaining compile errors. + +- [ ] **Step 2: Run clippy** + +```bash +cargo clippy -- -D warnings 2>&1 +``` + +Expected: no warnings. Fix any clippy lints before continuing. + +- [ ] **Step 3: Run the full benchmark suite to confirm no panics** + +```bash +cargo bench --bench minigraf_bench 2>&1 | tail -30 +``` + +Expected: all benchmarks complete without panic. + +- [ ] **Step 4: Open the PR** + +```bash +gh pr create \ + --repo project-minigraf/minigraf \ + --title "perf: predicate push-down for Expr clauses + mixed rule optimization (#207, #206)" \ + --body "$(cat <<'EOF' +## Summary + +- **#207**: Extends `optimizer::plan()` to accept `Vec` (Pattern + Expr) and return an interleaved plan. Each `Expr` predicate clause is positioned at the earliest point where all its variables are bound by preceding patterns, rather than being applied as a post-pass over the full binding set. +- **#206**: Routes the `StratifiedEvaluator` mixed-rules path through the updated `plan()`, giving mixed rules both selectivity-based pattern ordering and Expr push-down with no separate implementation. + +## New helper + +`PatternMatcher::match_with_hint_seeded()` — used by both executor and evaluator to process the plan incrementally (first pattern uses index hint; subsequent patterns use hash-join). + +## Test plan + +- [ ] `cargo test` — all tests pass +- [ ] `cargo clippy -- -D warnings` — no warnings +- [ ] `cargo bench --bench minigraf_bench -- query/predicate_pushdown` — new benchmark runs without panic + +## Follow-up + +- Not/NotJoin push-down (same mechanism, more complex safety rules): project-minigraf/minigraf#248 + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +- [ ] **Step 5: Monitor CI and fix any failures** + +```bash +gh pr checks --watch +``` + +Expected: all checks green. If any fail, diagnose and fix before merging. From 362502ee54e442c567d00723924c546f9f96bb76 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Fri, 15 May 2026 23:07:48 +0530 Subject: [PATCH 03/17] docs: add design spec for Wave 2 PR 2 cost-based optimizer extensions (#205) Co-Authored-By: Claude Sonnet 4.6 --- .../2026-05-15-cost-based-optimizer-design.md | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md diff --git a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md new file mode 100644 index 000000000..b6a7ffb37 --- /dev/null +++ b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md @@ -0,0 +1,143 @@ +# Design: Cost-Based Optimizer Extensions (Wave 2 PR 2) + +**Issue**: #205 (cost-based optimizer extensions for new clause types) +**Date**: 2026-05-15 +**Branch**: single worktree + PR + +--- + +## Summary + +Extend `optimizer.rs` with a static cardinality cost model and use it to order `not`/`not-join` clauses and `or`/`or-join` branches by estimated evaluation cost. Cheap filters run first; expensive ones only process the bindings that survived earlier filters. + +Aggregation post-processing cost estimation is **out of scope** — aggregation always runs last and cannot be reordered. + +Follow-up issue for Or short-circuit optimization (which builds on this cost model): #250. + +--- + +## Current Behaviour + +- `not`/`not-join` clauses are applied in source order — an expensive not-clause that scans many facts runs before a cheap one, wasting work on bindings that would have been rejected cheaply. +- `or`/`or-join` branches are evaluated in source order — no cost-based selection. +- The optimizer has no concept of cost for clause types other than `Pattern` and `Expr`. + +--- + +## Design + +### 1. Cost model — `optimizer.rs` + +#### `pattern_cost(p: &Pattern) -> u64` (private) + +Static 4-tier model derived from the existing `selectivity_score()`: + +| Bound EAV components | Cost | +|---|---| +| 3 (all bound) | 1 | +| 2 | 10 | +| 1 | 100 | +| 0 (nothing bound) | 10,000 | + +#### `pub fn clause_cost(clause: &WhereClause) -> u64` + +| Clause type | Cost formula | +|---|---| +| `Pattern(p)` | `pattern_cost(p)` | +| `Expr { .. }` | 0 (pure computation, no fact scan) | +| `Not(body)` | `min(pattern_cost(p) for p in body patterns)`, or 0 if body has no patterns | +| `NotJoin { clauses, .. }` | same as `Not` using `clauses` | +| `Or(branches)` | `sum(branch_cost(b) for b in branches)` | +| `OrJoin { branches, .. }` | same as `Or` | +| other | `u64::MAX` (never passed; defensive) | + +#### `pub fn branch_cost(branch: &[WhereClause]) -> u64` + +`min(pattern_cost(p) for p in branch patterns)`, or 0 if branch has no patterns. + +**Rationale for `min` in not/or bodies**: In a multi-pattern join, the most selective pattern (lowest cardinality) dominates — the join can never produce more rows than the smallest input. Using `min` avoids overestimating the cost of a body that contains at least one highly selective pattern. + +**WASM behaviour**: `clause_cost()` and `branch_cost()` are unconditional (available on all targets). The *sorting* call sites are gated behind `#[cfg(not(feature = "wasm"))]`, consistent with the existing pattern ordering gate. + +--- + +### 2. Executor changes — `executor.rs` + +#### `not`/`not-join` ordering + +In both `execute_query()` and `execute_query_with_rules()`, after extracting `not_clauses` and `not_join_clauses` and before the filter loop, sort each by `clause_cost()` ascending: + +```rust +#[cfg(not(feature = "wasm"))] +not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); + +#[cfg(not(feature = "wasm"))] +not_join_clauses.sort_by_key(|(vars, body)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: body.clone(), + }) +}); +``` + +**Effect**: Cheap not-filters run first. Bindings rejected by a cheap filter never reach more expensive filters. No change to semantics. + +#### `or`/`or-join` branch ordering + +In `apply_or_clauses()`, before evaluating branches in both the fast path (no Not in branches) and the slow path (seeded), sort branches by `branch_cost()` ascending: + +```rust +#[cfg(not(feature = "wasm"))] +let sorted_branches: Vec<_> = { + let mut b = branches.to_vec(); + b.sort_by_key(|br| optimizer::branch_cost(br)); + b +}; +// iterate sorted_branches instead of branches +``` + +**Note**: In the current executor, all Or branches are always evaluated (no short-circuit). Branch ordering is a no-op for correctness and has no measurable performance impact today. It is infrastructure for #250 (short-circuit optimization). A code comment explains this. + +--- + +### 3. Evaluator changes — `evaluator.rs` + +The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending, gated behind `#[cfg(not(feature = "wasm"))]`. The sort is identical to the executor call sites. + +--- + +### 4. Testing + +**`optimizer.rs` unit tests** — `clause_cost()` and `branch_cost()`: +- Fully-bound pattern → cost 1 +- 2-bound pattern → cost 10 +- 1-bound pattern → cost 100 +- Unbound pattern → cost 10,000 +- `Not` body: one selective + one full-scan pattern → cost = 1 (min) +- `Not` body with no patterns (expr-only) → cost 0 +- `Or` with two branches → cost = sum of branch min-costs +- `branch_cost` of empty branch → 0 + +**`executor.rs` integration tests** — not ordering: +- Query with two `not` clauses in expensive-first source order → results identical to cheap-first order (correctness regression guard) +- Query with two `not` clauses where cheap one eliminates most bindings → results correct + +**Validation**: The existing `bench_negation` group in `benches/minigraf_bench.rs` provides the baseline for measuring `not`-ordering improvement. No new benchmark group is added in this PR. + +--- + +## Invariants Preserved + +- **Correctness**: Sorting only changes evaluation order, never the set of bindings that satisfy a clause. The `not` filter is semantically commutative when applied to independent not-bodies over the same outer binding set. +- **WASM portability**: Cost functions unconditional; sorting gated behind `#[cfg(not(feature = "wasm"))]`. +- **Backwards compatibility**: No change to `plan()` signature or return type. `clause_cost()` and `branch_cost()` are new public additions. + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `src/query/datalog/optimizer.rs` | New `pattern_cost()`, `clause_cost()`, `branch_cost()` | +| `src/query/datalog/executor.rs` | Cost-sorted `not_clauses`, `not_join_clauses` in both query functions; cost-sorted branches in `apply_or_clauses()` | +| `src/query/datalog/evaluator.rs` | Cost-sorted `not_clauses`, `not_join_clauses` in mixed-rules loop | From 6aa8466ca03fb550c5c9daf19fdd3ab295f0d378 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 08:37:22 +0530 Subject: [PATCH 04/17] docs: explain WASM ordering gate at each cfg site in optimizer and spec Add a concise 'WASM omission' comment at every existing and planned #[cfg(not(feature = "wasm"))] ordering gate explaining why sorting is disabled on WASM targets (small datasets, deterministic behaviour). The full rationale lives in the selectivity_score() doc comment; all other sites reference it with a short cross-reference. Co-Authored-By: Claude Sonnet 4.6 --- .../2026-05-15-cost-based-optimizer-design.md | 24 ++++++++++++++++--- src/query/datalog/optimizer.rs | 5 ++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md index b6a7ffb37..e972828a2 100644 --- a/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md +++ b/docs/superpowers/specs/2026-05-15-cost-based-optimizer-design.md @@ -68,9 +68,11 @@ Static 4-tier model derived from the existing `selectivity_score()`: In both `execute_query()` and `execute_query_with_rules()`, after extracting `not_clauses` and `not_join_clauses` and before the filter loop, sort each by `clause_cost()` ascending: ```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] not_join_clauses.sort_by_key(|(vars, body)| { optimizer::clause_cost(&WhereClause::NotJoin { @@ -87,6 +89,9 @@ not_join_clauses.sort_by_key(|(vars, body)| { In `apply_or_clauses()`, before evaluating branches in both the fast path (no Not in branches) and the slow path (seeded), sort branches by `branch_cost()` ascending: ```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +// Note: all branches are still evaluated (no short-circuit); this ordering is +// infrastructure for #250. See that issue for the short-circuit optimization. #[cfg(not(feature = "wasm"))] let sorted_branches: Vec<_> = { let mut b = branches.to_vec(); @@ -96,13 +101,26 @@ let sorted_branches: Vec<_> = { // iterate sorted_branches instead of branches ``` -**Note**: In the current executor, all Or branches are always evaluated (no short-circuit). Branch ordering is a no-op for correctness and has no measurable performance impact today. It is infrastructure for #250 (short-circuit optimization). A code comment explains this. - --- ### 3. Evaluator changes — `evaluator.rs` -The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending, gated behind `#[cfg(not(feature = "wasm"))]`. The sort is identical to the executor call sites. +The `StratifiedEvaluator` mixed-rules loop applies `not`/`not-join` as post-filters in the `'binding:` loop. After extracting `not_clauses` and `not_join_clauses` (which already happens before the `'binding:` loop), sort both collections by `clause_cost()` ascending: + +```rust +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +#[cfg(not(feature = "wasm"))] +not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.clone()))); + +// WASM omission: small datasets + determinism — see optimizer::selectivity_score(). +#[cfg(not(feature = "wasm"))] +not_join_clauses.sort_by_key(|(vars, clauses)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: clauses.clone(), + }) +}); +``` --- diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index a741b984e..5648a0a53 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -39,6 +39,10 @@ fn attr_is_index_bound(a: &AttributeSpec) -> bool { /// Count the number of non-variable components in a pattern. /// Higher score = more selective. +/// +/// Gated on non-wasm: WASM/browser targets typically work with small datasets where +/// the overhead of computing scores and sorting can equal or exceed the benefit. +/// Source order is preserved on WASM for deterministic, debuggable query behaviour. #[cfg(not(feature = "wasm"))] fn selectivity_score(p: &Pattern) -> u8 { let e = !is_variable(&p.entity); @@ -145,6 +149,7 @@ pub fn plan( // Stable sort patterns by selectivity descending (non-wasm only). // Preserves original order for ties, ensuring deterministic output. + // WASM omission: see selectivity_score() — small datasets, determinism. #[cfg(not(feature = "wasm"))] patterns.sort_by_key(|(clause, _)| { if let WhereClause::Pattern(p) = clause { From ab01db10bb800b9c824049689c135e3475d4bd52 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:00:58 +0530 Subject: [PATCH 05/17] test(optimizer): failing tests for pattern_cost, branch_cost, clause_cost (#205) Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/optimizer.rs | 134 +++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index 5648a0a53..81eb8d95b 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -456,4 +456,138 @@ mod tests { "Expr with unbound var must be last" ); } + + // ── cost model tests ────────────────────────────────────────────────── + + #[test] + fn test_pattern_cost_fully_bound() { + // entity bound (UUID), attribute real keyword, value bound literal — 3 bound → cost 1 + let p = Pattern::new( + EdnValue::Uuid(Uuid::new_v4()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + assert_eq!(pattern_cost(&p), 1); + } + + #[test] + fn test_pattern_cost_two_bound() { + // attribute + value bound, entity variable — 2 bound → cost 10 + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + assert_eq!(pattern_cost(&p), 10); + } + + #[test] + fn test_pattern_cost_one_bound() { + // only attribute bound — 1 bound → cost 100 + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + assert_eq!(pattern_cost(&p), 100); + } + + #[test] + fn test_pattern_cost_unbound() { + // all variables — 0 bound → cost 10_000 + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Symbol("?a".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + assert_eq!(pattern_cost(&p), 10_000); + } + + #[test] + fn test_clause_cost_pattern_two_bound() { + // clause_cost delegates to pattern_cost for Pattern variant + // attr + value bound = 2 → cost 10 + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + assert_eq!(clause_cost(&WhereClause::Pattern(p)), 10); + } + + #[test] + fn test_clause_cost_expr_is_zero() { + // Expr is pure computation — cost 0 + let clause = WhereClause::Expr { + expr: Expr::Lit(Value::Integer(42)), + binding: None, + }; + assert_eq!(clause_cost(&clause), 0); + } + + #[test] + fn test_clause_cost_not_body_uses_min() { + // Not body: one cost-10 pattern + one cost-10_000 pattern → min = 10 + let selective = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + let full_scan = Pattern::new( + EdnValue::Symbol("?x".to_string()), + EdnValue::Symbol("?a".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + let clause = WhereClause::Not(vec![ + WhereClause::Pattern(selective), + WhereClause::Pattern(full_scan), + ]); + assert_eq!(clause_cost(&clause), 10); + } + + #[test] + fn test_clause_cost_not_body_expr_only_is_zero() { + // Not body with no patterns (expr only) → cost 0 + let clause = WhereClause::Not(vec![WhereClause::Expr { + expr: Expr::Lit(Value::Integer(1)), + binding: None, + }]); + assert_eq!(clause_cost(&clause), 0); + } + + #[test] + fn test_branch_cost_empty_branch() { + // Empty branch → 0 + assert_eq!(branch_cost(&[]), 0); + } + + #[test] + fn test_branch_cost_expr_only_is_zero() { + // Branch with only Expr clauses → 0 + let branch = vec![WhereClause::Expr { + expr: Expr::Lit(Value::Integer(99)), + binding: None, + }]; + assert_eq!(branch_cost(&branch), 0); + } + + #[test] + fn test_clause_cost_or_sums_branch_costs() { + // Or with two branches: + // branch 1: one pattern with cost 10 (attr+value bound) + // branch 2: one pattern with cost 100 (attr only bound) + // clause_cost(Or) = sum = 110 + let b1 = vec![WhereClause::Pattern(Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ))]; + let b2 = vec![WhereClause::Pattern(Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/age".to_string()), + EdnValue::Symbol("?v".to_string()), + ))]; + let clause = WhereClause::Or(vec![b1, b2]); + assert_eq!(clause_cost(&clause), 110); // 10 + 100 + } } From f985b2b151e6de2b432e076ab5f9e3c51e10ad9c Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:02:53 +0530 Subject: [PATCH 06/17] feat(optimizer): add pattern_cost, branch_cost, clause_cost (#205) Static 4-tier cardinality cost model. Unconditional (all targets). Sorting call-sites gated on cfg(not(feature = "wasm")) added in next commit. --- src/query/datalog/optimizer.rs | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index 81eb8d95b..bd67fb774 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -196,6 +196,70 @@ pub fn plan( result } +/// Static 4-tier cardinality estimate for a single pattern. +/// +/// Derived from selectivity_score but returns u64 cost (lower = cheaper) rather +/// than a selectivity score. Unconditional: available on all targets including WASM. +fn pattern_cost(p: &Pattern) -> u64 { + let e = !is_variable(&p.entity); + let a = attr_is_index_bound(&p.attribute); + let v = !is_variable(&p.value); + match (e as u8) + (a as u8) + (v as u8) { + 3 => 1, + 2 => 10, + 1 => 100, + _ => 10_000, + } +} + +/// Estimated cost for a body/branch slice — the minimum `pattern_cost` across all +/// Pattern clauses, or 0 if the body contains no patterns (expr-only bodies are +/// cheap pure computation). +/// +/// Rationale for `min`: In a multi-pattern join the most selective pattern dominates — +/// the join cannot produce more rows than the smallest input. +/// +/// Unconditional: available on all targets including WASM. +pub fn branch_cost(branch: &[WhereClause]) -> u64 { + branch + .iter() + .filter_map(|c| { + if let WhereClause::Pattern(p) = c { + Some(pattern_cost(p)) + } else { + None + } + }) + .min() + .unwrap_or(0) +} + +/// Estimated evaluation cost for any `WhereClause`. +/// +/// | Clause type | Cost | +/// |--------------------|------| +/// | `Pattern` | `pattern_cost(p)` | +/// | `Expr` | 0 (pure computation) | +/// | `Not(body)` | `branch_cost(body)` | +/// | `NotJoin{clauses}` | `branch_cost(clauses)` | +/// | `Or(branches)` | sum of `branch_cost` per branch | +/// | `OrJoin{branches}` | sum of `branch_cost` per branch | +/// | other | `u64::MAX` (defensive; not expected in practice) | +/// +/// Unconditional: available on all targets including WASM. The *sorting* call-sites +/// that consume this function are gated behind `#[cfg(not(feature = "wasm"))]`. +pub fn clause_cost(clause: &WhereClause) -> u64 { + match clause { + WhereClause::Pattern(p) => pattern_cost(p), + WhereClause::Expr { .. } => 0, + WhereClause::Not(body) => branch_cost(body), + WhereClause::NotJoin { clauses, .. } => branch_cost(clauses), + WhereClause::Or(branches) => branches.iter().map(|b| branch_cost(b)).sum(), + WhereClause::OrJoin { branches, .. } => branches.iter().map(|b| branch_cost(b)).sum(), + _ => u64::MAX, + } +} + #[cfg(test)] mod tests { use super::*; From 66dfaff73512f02e16392e5664810b8201039281 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:05:26 +0530 Subject: [PATCH 07/17] test(optimizer): add NotJoin and OrJoin coverage for clause_cost (#205) --- src/query/datalog/optimizer.rs | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index bd67fb774..f3adb80e6 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -654,4 +654,39 @@ mod tests { let clause = WhereClause::Or(vec![b1, b2]); assert_eq!(clause_cost(&clause), 110); // 10 + 100 } + + #[test] + fn test_clause_cost_not_join_uses_branch_cost() { + // NotJoin with one selective pattern (cost 10) → cost 10 + let p = Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + let clause = WhereClause::NotJoin { + join_vars: vec!["?e".to_string()], + clauses: vec![WhereClause::Pattern(p)], + }; + assert_eq!(clause_cost(&clause), 10); + } + + #[test] + fn test_clause_cost_or_join_sums_branch_costs() { + // OrJoin with two branches: cost 10 + cost 100 = 110 + let b1 = vec![WhereClause::Pattern(Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ))]; + let b2 = vec![WhereClause::Pattern(Pattern::new( + EdnValue::Symbol("?e".to_string()), + EdnValue::Keyword(":person/age".to_string()), + EdnValue::Symbol("?v".to_string()), + ))]; + let clause = WhereClause::OrJoin { + join_vars: vec!["?e".to_string()], + branches: vec![b1, b2], + }; + assert_eq!(clause_cost(&clause), 110); + } } From cb65cad26f53167e4753164344869789f8de0bb6 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:07:44 +0530 Subject: [PATCH 08/17] test(optimizer): add cost-1 propagation test through branch_cost/clause_cost (#205) Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/optimizer.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index f3adb80e6..d54c22b7e 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -689,4 +689,25 @@ mod tests { }; assert_eq!(clause_cost(&clause), 110); } + + #[test] + fn test_clause_cost_not_body_fully_bound_min_is_one() { + // Not body: one fully-bound pattern (cost 1) + one full-scan (cost 10_000) + // clause_cost → min = 1 + let fully_bound = Pattern::new( + EdnValue::Uuid(Uuid::new_v4()), + EdnValue::Keyword(":person/name".to_string()), + EdnValue::String("Alice".to_string()), + ); + let full_scan = Pattern::new( + EdnValue::Symbol("?x".to_string()), + EdnValue::Symbol("?a".to_string()), + EdnValue::Symbol("?v".to_string()), + ); + let clause = WhereClause::Not(vec![ + WhereClause::Pattern(full_scan), + WhereClause::Pattern(fully_bound), + ]); + assert_eq!(clause_cost(&clause), 1); + } } From 29dd293c3d5ef6f4966a51d5cf693bcc4b2705b2 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:11:37 +0530 Subject: [PATCH 09/17] feat(executor): sort not/not-join clauses by cost ascending (#205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cheap filters run first; bindings rejected early never reach expensive filters. Gated on cfg(not(feature = "wasm")) — small datasets + determinism on WASM. --- src/query/datalog/executor.rs | 70 +++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 77a074fee..77e9832cb 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -571,7 +571,7 @@ impl DatalogExecutor { // Apply not-filter for WhereClause::Not and WhereClause::NotJoin clauses // (no rules involved — pure post-filter) - let not_clauses: Vec<&Vec> = query + let mut not_clauses: Vec<&Vec> = query .where_clauses .iter() .filter_map(|c| match c { @@ -580,7 +580,7 @@ impl DatalogExecutor { }) .collect(); - let not_join_clauses: Vec<(Vec, Vec)> = query + let mut not_join_clauses: Vec<(Vec, Vec)> = query .where_clauses .iter() .filter_map(|c| match c { @@ -591,6 +591,13 @@ impl DatalogExecutor { }) .collect(); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); + let not_filtered: Vec<_> = if not_clauses.is_empty() && not_join_clauses.is_empty() { bindings } else { @@ -1039,7 +1046,7 @@ impl DatalogExecutor { // in the query body. (The StratifiedEvaluator handles `not`/`not-join` in rule // bodies; this handles them appearing directly in the query body alongside rule // invocations.) - let not_clauses: Vec<&Vec> = query + let mut not_clauses: Vec<&Vec> = query .where_clauses .iter() .filter_map(|c| match c { @@ -1048,7 +1055,7 @@ impl DatalogExecutor { }) .collect(); - let not_join_clauses: Vec<(Vec, Vec)> = query + let mut not_join_clauses: Vec<(Vec, Vec)> = query .where_clauses .iter() .filter_map(|c| match c { @@ -1059,6 +1066,13 @@ impl DatalogExecutor { }) .collect(); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); + let not_filtered: Vec<_> = if not_clauses.is_empty() && not_join_clauses.is_empty() { bindings } else { @@ -5421,6 +5435,7 @@ mod or_hash_join_tests { #[cfg(test)] mod pushdown_tests { use crate::graph::FactStorage; + use crate::graph::types::Value; use crate::query::datalog::executor::{DatalogExecutor, QueryResult}; use crate::query::datalog::parser::parse_datalog_command; @@ -5518,4 +5533,51 @@ mod pushdown_tests { panic!("expected QueryResults"); } } + + #[test] + fn test_not_clause_ordering_correctness() { + // Two `not` clauses given in expensive-first source order; results must be + // identical regardless of which order the optimizer chooses to evaluate them. + // This is a correctness regression guard — semantics must not change. + let storage = FactStorage::new(); + let executor = DatalogExecutor::new(storage); + + // Transact 3 items: widget, gadget, doohickey + executor + .execute( + parse_datalog_command( + r#"(transact [[:item1 :item/name "widget"] + [:item2 :item/name "gadget"] + [:item3 :item/name "doohickey"]])"#, + ) + .unwrap(), + ) + .unwrap(); + + // Query: find items that are NOT "widget" AND NOT "gadget" + // Clauses are given in expensive-first order to exercise the cost-based sort. + let result = executor + .execute( + parse_datalog_command( + r#"(query [:find ?name + :where [?e :item/name ?name] + (not [?e :item/name "gadget"]) + (not [?e :item/name "widget"])])"#, + ) + .unwrap(), + ) + .unwrap(); + + match result { + QueryResult::QueryResults { results, .. } => { + assert_eq!(results.len(), 1, "only doohickey should pass"); + assert_eq!( + results[0][0], + Value::String("doohickey".to_string()), + "result should be doohickey" + ); + } + _ => panic!("expected QueryResults"), + } + } } From fa1d92c439b8269478df3161aa52d49793001d68 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:15:11 +0530 Subject: [PATCH 10/17] test(executor): fix CodeQL assert_eq on Value in not-ordering test (#205) --- src/query/datalog/executor.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 77e9832cb..11ea09030 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -5571,11 +5571,11 @@ mod pushdown_tests { match result { QueryResult::QueryResults { results, .. } => { assert_eq!(results.len(), 1, "only doohickey should pass"); - assert_eq!( - results[0][0], - Value::String("doohickey".to_string()), - "result should be doohickey" - ); + if let Value::String(ref s) = results[0][0] { + assert_eq!(s.as_str(), "doohickey", "result should be doohickey"); + } else { + panic!("expected a String value for the result"); + } } _ => panic!("expected QueryResults"), } From efe3e5d22087aa7947df6c157a3e3ef009803d0a Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:17:51 +0530 Subject: [PATCH 11/17] feat(executor): sort or/or-join branches by cost ascending (#205) Infrastructure for issue #250 (or short-circuit). All branches still evaluated. Gated on cfg(not(feature = "wasm")). Also adds #[allow(dead_code)] to clause_cost which is public API exercised in tests but not yet consumed by non-test call-sites. Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/executor.rs | 28 ++++++++++++++++++++++++---- src/query/datalog/optimizer.rs | 1 + 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 11ea09030..8fe03af5a 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -1844,10 +1844,20 @@ pub(crate) fn apply_or_clauses( for clause in clauses { match clause { WhereClause::Or(branches) => { + let sorted_or_branches: Vec<&Vec> = { + let mut b: Vec<&Vec> = branches.iter().collect(); + // Sort branches by cost ascending so cheaper branches evaluate first. + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + // Note: all branches still evaluated (no short-circuit); ordering is + // infrastructure for issue #250. + #[cfg(not(feature = "wasm"))] + b.sort_by_key(|br| optimizer::branch_cost(br)); + b + }; // If any branch contains Not/NotJoin clauses (which need bound variables // from the outer scope to evaluate correctly), fall back to the classic // seeded-branch evaluation to preserve correctness. - let any_branch_has_not = branches.iter().any(|b| { + let any_branch_has_not = sorted_or_branches.iter().any(|b| { b.iter() .any(|c| matches!(c, WhereClause::Not(_) | WhereClause::NotJoin { .. })) }); @@ -1857,7 +1867,7 @@ pub(crate) fn apply_or_clauses( let mut seen: std::collections::HashSet> = std::collections::HashSet::new(); let mut result: Vec = Vec::new(); - for branch in branches { + for branch in &sorted_or_branches { let branch_result = evaluate_branch( branch, bindings.clone(), @@ -1891,7 +1901,7 @@ pub(crate) fn apply_or_clauses( let mut seen_keys: std::collections::HashSet> = std::collections::HashSet::new(); - for branch in branches { + for branch in &sorted_or_branches { let branch_result = evaluate_branch( branch, empty_seed.clone(), @@ -1989,6 +1999,16 @@ pub(crate) fn apply_or_clauses( join_vars, branches, } => { + let sorted_oj_branches: Vec<&Vec> = { + let mut b: Vec<&Vec> = branches.iter().collect(); + // Sort branches by cost ascending so cheaper branches evaluate first. + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + // Note: all branches still evaluated (no short-circuit); ordering is + // infrastructure for issue #250. + #[cfg(not(feature = "wasm"))] + b.sort_by_key(|br| optimizer::branch_cost(br)); + b + }; let outer_keys: std::collections::HashSet = bindings.iter().flat_map(|b| b.keys().cloned()).collect(); @@ -2006,7 +2026,7 @@ pub(crate) fn apply_or_clauses( let mut seen_proj: std::collections::HashSet> = std::collections::HashSet::new(); - for branch in branches { + for branch in &sorted_oj_branches { let branch_result = evaluate_branch( branch, empty_seed.clone(), diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index d54c22b7e..a52dc5590 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -248,6 +248,7 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { /// /// Unconditional: available on all targets including WASM. The *sorting* call-sites /// that consume this function are gated behind `#[cfg(not(feature = "wasm"))]`. +#[allow(dead_code)] // public API; used in tests; will be consumed by sort call-sites in issue #250 pub fn clause_cost(clause: &WhereClause) -> u64 { match clause { WhereClause::Pattern(p) => pattern_cost(p), From 55636ea47d497bdabb301e4703c9560e051f95ef Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 09:22:41 +0530 Subject: [PATCH 12/17] chore(optimizer): remove unnecessary allow(dead_code) on pub fn clause_cost (#205) --- src/query/datalog/optimizer.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index a52dc5590..01c5cbf05 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -224,8 +224,8 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { branch .iter() .filter_map(|c| { - if let WhereClause::Pattern(p) = c { - Some(pattern_cost(p)) + if matches!(c, WhereClause::Pattern(_)) { + Some(clause_cost(c)) } else { None } @@ -248,7 +248,6 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { /// /// Unconditional: available on all targets including WASM. The *sorting* call-sites /// that consume this function are gated behind `#[cfg(not(feature = "wasm"))]`. -#[allow(dead_code)] // public API; used in tests; will be consumed by sort call-sites in issue #250 pub fn clause_cost(clause: &WhereClause) -> u64 { match clause { WhereClause::Pattern(p) => pattern_cost(p), From a0fc552ad27a341557536f3e8989c3ded13112c8 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 10:06:45 +0530 Subject: [PATCH 13/17] refactor(optimizer): restore branch_cost to idiomatic if-let form (#205) Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/optimizer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index 01c5cbf05..d54c22b7e 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -224,8 +224,8 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { branch .iter() .filter_map(|c| { - if matches!(c, WhereClause::Pattern(_)) { - Some(clause_cost(c)) + if let WhereClause::Pattern(p) = c { + Some(pattern_cost(p)) } else { None } From 611d22819e291ec1b9c9cee36230ec8f7afcc2b8 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 10:08:52 +0530 Subject: [PATCH 14/17] feat(evaluator): sort not/not-join clauses by cost in StratifiedEvaluator (#205) Consistent with executor.rs; cheap filters run first in mixed-rules loop. Gated on cfg(not(feature = "wasm")). --- src/query/datalog/evaluator.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/query/datalog/evaluator.rs b/src/query/datalog/evaluator.rs index 0e6a6a52e..6f2a5b1f2 100644 --- a/src/query/datalog/evaluator.rs +++ b/src/query/datalog/evaluator.rs @@ -766,7 +766,7 @@ impl StratifiedEvaluator { } } - let not_clauses: Vec> = rule + let mut not_clauses: Vec> = rule .body .iter() .filter_map(|c| match c { @@ -775,7 +775,7 @@ impl StratifiedEvaluator { }) .collect(); - let not_join_clauses: Vec<(Vec, Vec)> = rule + let mut not_join_clauses: Vec<(Vec, Vec)> = rule .body .iter() .filter_map(|c| match c { @@ -786,6 +786,15 @@ impl StratifiedEvaluator { }) .collect(); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_clauses.sort_by_key(|body| crate::query::datalog::optimizer::branch_cost(body)); + // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). + #[cfg(not(feature = "wasm"))] + not_join_clauses.sort_by_key(|(_, clauses)| { + crate::query::datalog::optimizer::branch_cost(clauses) + }); + // Compute once; reuse for plan loop, apply_or_clauses, not-body matching. let accumulated_facts: Arc<[Fact]> = Arc::from(accumulated.get_asserted_facts()?); From f5c0bb6002a26fa25656db3c157edb1b37bf376d Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 10:36:01 +0530 Subject: [PATCH 15/17] refactor(executor,evaluator): use clause_cost at not-clause sort sites (#205) Makes clause_cost reachable from production code (fixes dead_code lint). Semantically identical: clause_cost(Not(body)) == branch_cost(body). --- src/query/datalog/evaluator.rs | 11 ++++++++--- src/query/datalog/executor.rs | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/query/datalog/evaluator.rs b/src/query/datalog/evaluator.rs index 6f2a5b1f2..7a644a865 100644 --- a/src/query/datalog/evaluator.rs +++ b/src/query/datalog/evaluator.rs @@ -788,11 +788,16 @@ impl StratifiedEvaluator { // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_clauses.sort_by_key(|body| crate::query::datalog::optimizer::branch_cost(body)); + not_clauses.sort_by_key(|body| { + crate::query::datalog::optimizer::clause_cost(&WhereClause::Not(body.clone())) + }); // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_join_clauses.sort_by_key(|(_, clauses)| { - crate::query::datalog::optimizer::branch_cost(clauses) + not_join_clauses.sort_by_key(|(vars, clauses)| { + crate::query::datalog::optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: clauses.clone(), + }) }); // Compute once; reuse for plan loop, apply_or_clauses, not-body matching. diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 8fe03af5a..4db56e855 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -593,10 +593,15 @@ impl DatalogExecutor { // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.to_vec()))); // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); + not_join_clauses.sort_by_key(|(vars, clauses)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: clauses.clone(), + }) + }); let not_filtered: Vec<_> = if not_clauses.is_empty() && not_join_clauses.is_empty() { bindings @@ -1068,10 +1073,15 @@ impl DatalogExecutor { // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_clauses.sort_by_key(|body| optimizer::branch_cost(body)); + not_clauses.sort_by_key(|body| optimizer::clause_cost(&WhereClause::Not(body.to_vec()))); // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). #[cfg(not(feature = "wasm"))] - not_join_clauses.sort_by_key(|(_, clauses)| optimizer::branch_cost(clauses)); + not_join_clauses.sort_by_key(|(vars, clauses)| { + optimizer::clause_cost(&WhereClause::NotJoin { + join_vars: vars.clone(), + clauses: clauses.clone(), + }) + }); let not_filtered: Vec<_> = if not_clauses.is_empty() && not_join_clauses.is_empty() { bindings From 4b0fb34ce27c5c63fc45f94d9a16d3028d65bda3 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 10:49:20 +0530 Subject: [PATCH 16/17] fix: resolve clippy warnings under --all-features (wasm + dead_code + unused_mut) Gate pattern_cost / branch_cost / clause_cost and their tests behind #[cfg(not(feature = "wasm"))] to match the call-sites that consume them. Add #[cfg_attr(feature = "wasm", allow(unused_mut))] to `let mut` bindings that are only mutated inside #[cfg(not(feature = "wasm"))] sort blocks. Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/evaluator.rs | 2 ++ src/query/datalog/executor.rs | 6 ++++++ src/query/datalog/optimizer.rs | 27 +++++++++++++++++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/query/datalog/evaluator.rs b/src/query/datalog/evaluator.rs index 7a644a865..5de1ce537 100644 --- a/src/query/datalog/evaluator.rs +++ b/src/query/datalog/evaluator.rs @@ -766,6 +766,7 @@ impl StratifiedEvaluator { } } + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_clauses: Vec> = rule .body .iter() @@ -775,6 +776,7 @@ impl StratifiedEvaluator { }) .collect(); + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_join_clauses: Vec<(Vec, Vec)> = rule .body .iter() diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 4db56e855..a8d88405f 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -571,6 +571,7 @@ impl DatalogExecutor { // Apply not-filter for WhereClause::Not and WhereClause::NotJoin clauses // (no rules involved — pure post-filter) + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_clauses: Vec<&Vec> = query .where_clauses .iter() @@ -580,6 +581,7 @@ impl DatalogExecutor { }) .collect(); + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_join_clauses: Vec<(Vec, Vec)> = query .where_clauses .iter() @@ -1051,6 +1053,7 @@ impl DatalogExecutor { // in the query body. (The StratifiedEvaluator handles `not`/`not-join` in rule // bodies; this handles them appearing directly in the query body alongside rule // invocations.) + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_clauses: Vec<&Vec> = query .where_clauses .iter() @@ -1060,6 +1063,7 @@ impl DatalogExecutor { }) .collect(); + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut not_join_clauses: Vec<(Vec, Vec)> = query .where_clauses .iter() @@ -1855,6 +1859,7 @@ pub(crate) fn apply_or_clauses( match clause { WhereClause::Or(branches) => { let sorted_or_branches: Vec<&Vec> = { + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut b: Vec<&Vec> = branches.iter().collect(); // Sort branches by cost ascending so cheaper branches evaluate first. // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). @@ -2010,6 +2015,7 @@ pub(crate) fn apply_or_clauses( branches, } => { let sorted_oj_branches: Vec<&Vec> = { + #[cfg_attr(feature = "wasm", allow(unused_mut))] let mut b: Vec<&Vec> = branches.iter().collect(); // Sort branches by cost ascending so cheaper branches evaluate first. // WASM omission: small datasets + determinism — see optimizer::selectivity_score(). diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index d54c22b7e..c4892da80 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -199,7 +199,8 @@ pub fn plan( /// Static 4-tier cardinality estimate for a single pattern. /// /// Derived from selectivity_score but returns u64 cost (lower = cheaper) rather -/// than a selectivity score. Unconditional: available on all targets including WASM. +/// than a selectivity score. Gated on non-wasm (only called from non-wasm paths). +#[cfg(not(feature = "wasm"))] fn pattern_cost(p: &Pattern) -> u64 { let e = !is_variable(&p.entity); let a = attr_is_index_bound(&p.attribute); @@ -219,7 +220,8 @@ fn pattern_cost(p: &Pattern) -> u64 { /// Rationale for `min`: In a multi-pattern join the most selective pattern dominates — /// the join cannot produce more rows than the smallest input. /// -/// Unconditional: available on all targets including WASM. +/// Gated on non-wasm: only called from non-wasm code paths. +#[cfg(not(feature = "wasm"))] pub fn branch_cost(branch: &[WhereClause]) -> u64 { branch .iter() @@ -246,8 +248,9 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { /// | `OrJoin{branches}` | sum of `branch_cost` per branch | /// | other | `u64::MAX` (defensive; not expected in practice) | /// -/// Unconditional: available on all targets including WASM. The *sorting* call-sites -/// that consume this function are gated behind `#[cfg(not(feature = "wasm"))]`. +/// Gated on non-wasm: only called from non-wasm code paths (sorting call-sites are +/// gated behind `#[cfg(not(feature = "wasm"))]`). +#[cfg(not(feature = "wasm"))] pub fn clause_cost(clause: &WhereClause) -> u64 { match clause { WhereClause::Pattern(p) => pattern_cost(p), @@ -522,7 +525,10 @@ mod tests { } // ── cost model tests ────────────────────────────────────────────────── + // These tests call pattern_cost / branch_cost / clause_cost which are + // gated on #[cfg(not(feature = "wasm"))]. + #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_fully_bound() { // entity bound (UUID), attribute real keyword, value bound literal — 3 bound → cost 1 @@ -534,6 +540,7 @@ mod tests { assert_eq!(pattern_cost(&p), 1); } + #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_two_bound() { // attribute + value bound, entity variable — 2 bound → cost 10 @@ -545,6 +552,7 @@ mod tests { assert_eq!(pattern_cost(&p), 10); } + #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_one_bound() { // only attribute bound — 1 bound → cost 100 @@ -556,6 +564,7 @@ mod tests { assert_eq!(pattern_cost(&p), 100); } + #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_unbound() { // all variables — 0 bound → cost 10_000 @@ -567,6 +576,7 @@ mod tests { assert_eq!(pattern_cost(&p), 10_000); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_pattern_two_bound() { // clause_cost delegates to pattern_cost for Pattern variant @@ -579,6 +589,7 @@ mod tests { assert_eq!(clause_cost(&WhereClause::Pattern(p)), 10); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_expr_is_zero() { // Expr is pure computation — cost 0 @@ -589,6 +600,7 @@ mod tests { assert_eq!(clause_cost(&clause), 0); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_uses_min() { // Not body: one cost-10 pattern + one cost-10_000 pattern → min = 10 @@ -609,6 +621,7 @@ mod tests { assert_eq!(clause_cost(&clause), 10); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_expr_only_is_zero() { // Not body with no patterns (expr only) → cost 0 @@ -619,12 +632,14 @@ mod tests { assert_eq!(clause_cost(&clause), 0); } + #[cfg(not(feature = "wasm"))] #[test] fn test_branch_cost_empty_branch() { // Empty branch → 0 assert_eq!(branch_cost(&[]), 0); } + #[cfg(not(feature = "wasm"))] #[test] fn test_branch_cost_expr_only_is_zero() { // Branch with only Expr clauses → 0 @@ -635,6 +650,7 @@ mod tests { assert_eq!(branch_cost(&branch), 0); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_or_sums_branch_costs() { // Or with two branches: @@ -655,6 +671,7 @@ mod tests { assert_eq!(clause_cost(&clause), 110); // 10 + 100 } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_join_uses_branch_cost() { // NotJoin with one selective pattern (cost 10) → cost 10 @@ -670,6 +687,7 @@ mod tests { assert_eq!(clause_cost(&clause), 10); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_or_join_sums_branch_costs() { // OrJoin with two branches: cost 10 + cost 100 = 110 @@ -690,6 +708,7 @@ mod tests { assert_eq!(clause_cost(&clause), 110); } + #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_fully_bound_min_is_one() { // Not body: one fully-bound pattern (cost 1) + one full-scan (cost 10_000) From c626b8c12437aa27c50c89e5d3e0cec2a1ff53bf Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Sat, 16 May 2026 11:01:27 +0530 Subject: [PATCH 17/17] =?UTF-8?q?fix(optimizer):=20restore=20cost=20functi?= =?UTF-8?q?ons=20to=20unconditional=20=E2=80=94=20use=20cfg=5Fattr=20allow?= =?UTF-8?q?=20instead=20of=20cfg=20gate=20(#205)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec requires pattern_cost/branch_cost/clause_cost to be available on all targets. Only the sorting call-sites are gated on cfg(not(feature = "wasm")). Use cfg_attr(feature = "wasm", allow(dead_code)) to silence the dead_code lint on wasm. Cost-model unit tests also made unconditional (they don't run on wasm32 anyway). --- src/query/datalog/optimizer.rs | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/query/datalog/optimizer.rs b/src/query/datalog/optimizer.rs index c4892da80..d769a9c07 100644 --- a/src/query/datalog/optimizer.rs +++ b/src/query/datalog/optimizer.rs @@ -199,8 +199,9 @@ pub fn plan( /// Static 4-tier cardinality estimate for a single pattern. /// /// Derived from selectivity_score but returns u64 cost (lower = cheaper) rather -/// than a selectivity score. Gated on non-wasm (only called from non-wasm paths). -#[cfg(not(feature = "wasm"))] +/// than a selectivity score. Available on all targets; on WASM the dead_code lint +/// is suppressed because the sorting call-sites are omitted there. +#[cfg_attr(feature = "wasm", allow(dead_code))] fn pattern_cost(p: &Pattern) -> u64 { let e = !is_variable(&p.entity); let a = attr_is_index_bound(&p.attribute); @@ -220,8 +221,9 @@ fn pattern_cost(p: &Pattern) -> u64 { /// Rationale for `min`: In a multi-pattern join the most selective pattern dominates — /// the join cannot produce more rows than the smallest input. /// -/// Gated on non-wasm: only called from non-wasm code paths. -#[cfg(not(feature = "wasm"))] +/// Available on all targets; on WASM the dead_code lint is suppressed because +/// the sorting call-sites are omitted there. +#[cfg_attr(feature = "wasm", allow(dead_code))] pub fn branch_cost(branch: &[WhereClause]) -> u64 { branch .iter() @@ -248,9 +250,9 @@ pub fn branch_cost(branch: &[WhereClause]) -> u64 { /// | `OrJoin{branches}` | sum of `branch_cost` per branch | /// | other | `u64::MAX` (defensive; not expected in practice) | /// -/// Gated on non-wasm: only called from non-wasm code paths (sorting call-sites are -/// gated behind `#[cfg(not(feature = "wasm"))]`). -#[cfg(not(feature = "wasm"))] +/// Available on all targets; on WASM the dead_code lint is suppressed because +/// the sorting call-sites are omitted there. +#[cfg_attr(feature = "wasm", allow(dead_code))] pub fn clause_cost(clause: &WhereClause) -> u64 { match clause { WhereClause::Pattern(p) => pattern_cost(p), @@ -526,9 +528,8 @@ mod tests { // ── cost model tests ────────────────────────────────────────────────── // These tests call pattern_cost / branch_cost / clause_cost which are - // gated on #[cfg(not(feature = "wasm"))]. + // unconditional (available on all targets). - #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_fully_bound() { // entity bound (UUID), attribute real keyword, value bound literal — 3 bound → cost 1 @@ -540,7 +541,6 @@ mod tests { assert_eq!(pattern_cost(&p), 1); } - #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_two_bound() { // attribute + value bound, entity variable — 2 bound → cost 10 @@ -552,7 +552,6 @@ mod tests { assert_eq!(pattern_cost(&p), 10); } - #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_one_bound() { // only attribute bound — 1 bound → cost 100 @@ -564,7 +563,6 @@ mod tests { assert_eq!(pattern_cost(&p), 100); } - #[cfg(not(feature = "wasm"))] #[test] fn test_pattern_cost_unbound() { // all variables — 0 bound → cost 10_000 @@ -576,7 +574,6 @@ mod tests { assert_eq!(pattern_cost(&p), 10_000); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_pattern_two_bound() { // clause_cost delegates to pattern_cost for Pattern variant @@ -589,7 +586,6 @@ mod tests { assert_eq!(clause_cost(&WhereClause::Pattern(p)), 10); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_expr_is_zero() { // Expr is pure computation — cost 0 @@ -600,7 +596,6 @@ mod tests { assert_eq!(clause_cost(&clause), 0); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_uses_min() { // Not body: one cost-10 pattern + one cost-10_000 pattern → min = 10 @@ -621,7 +616,6 @@ mod tests { assert_eq!(clause_cost(&clause), 10); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_expr_only_is_zero() { // Not body with no patterns (expr only) → cost 0 @@ -632,14 +626,12 @@ mod tests { assert_eq!(clause_cost(&clause), 0); } - #[cfg(not(feature = "wasm"))] #[test] fn test_branch_cost_empty_branch() { // Empty branch → 0 assert_eq!(branch_cost(&[]), 0); } - #[cfg(not(feature = "wasm"))] #[test] fn test_branch_cost_expr_only_is_zero() { // Branch with only Expr clauses → 0 @@ -650,7 +642,6 @@ mod tests { assert_eq!(branch_cost(&branch), 0); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_or_sums_branch_costs() { // Or with two branches: @@ -671,7 +662,6 @@ mod tests { assert_eq!(clause_cost(&clause), 110); // 10 + 100 } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_join_uses_branch_cost() { // NotJoin with one selective pattern (cost 10) → cost 10 @@ -687,7 +677,6 @@ mod tests { assert_eq!(clause_cost(&clause), 10); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_or_join_sums_branch_costs() { // OrJoin with two branches: cost 10 + cost 100 = 110 @@ -708,7 +697,6 @@ mod tests { assert_eq!(clause_cost(&clause), 110); } - #[cfg(not(feature = "wasm"))] #[test] fn test_clause_cost_not_body_fully_bound_min_is_one() { // Not body: one fully-bound pattern (cost 1) + one full-scan (cost 10_000)