8000 feat(query): every build reorders; the wasm feature is a no-op (+ #606) by adityamukho · Pull Request #607 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **Join answers no longer depend on the query plan** (#545). Two cases changed. A join on a float value now pairs `0.0` with `-0.0`, as a shared variable compared after the join key already did; before, `Value`'s hash told them apart, so the join key never paired them. NaN still equals nothing. Float constants in a pattern still match within `f64::EPSILON`. And a variable bound to a keyword by one pattern and to the ref of the entity the keyword names by another, which already joined, now returns the kind from the first of those patterns in the `:where` text. Before, it returned the kind from the pattern the planner ran first, so the answer could change with the selectivity of the other patterns, and differed under the `wasm` feature, which does not reorder. For example, with `[:alice :friend :bob]` stored as a keyword, `[:find ?b :where [?a :friend ?b] [?b :name "Bob"]]` returned the ref of `:bob`, because the planner runs the pattern with two constants first, and now returns the keyword `:bob`. Expression bindings follow the next entry (#596). The rule covers the top-level `:where` of queries with and without rules; inside `or`, `not` and rule bodies the kinds are unchanged.
- **An expression binding a variable that another clause binds keeps only the rows where the two agree** (#596). `[(f …) ?x]` used to overwrite `?x` wherever the planner ran it, so when a pattern also bound `?x` the answer depended on whether that pattern ran first, and differed under the `wasm` feature, which does not reorder. Now the expression unifies: the row is kept when its result equals `?x`'s value, with the join equality of #545 (floats by IEEE `==`, a keyword equal to a ref to the entity it names, an integer never equal to a float, NaN equal to nothing). For example, with `[:a :age 30] [:b :age 40] [:p1 :score 41]`, `[:find ?a ?x :where [?e :age ?a] [:p1 :score ?x] [(+ ?a 1) ?x]]` returned `[30 31] [40 41]` natively and now returns `[40 41]`, as it did under `wasm`. `[(+ ?v 1) ?v]` never matches, and two expressions binding one variable must agree. When the two values differ only as keyword and ref, the variable returns the value of the first clause in the `:where` text that binds it, expressions included. This applies to `or` branches, `not` bodies and rule bodies too, on the executor and on the streaming engine.
- **A per-fact pseudo-attribute reads the nearest pattern before it with the same entity** (#594). `:db/valid-from`, `:db/valid-to`, `:db/tx-count` and `:db/tx-id` read the metadata of one fact: the one matched by their *source*, chosen when the query, rule or prepared query is parsed, as the nearest pattern before them in the `:where` text with the same entity term and a stored attribute (a keyword, or a variable as in `[?e ?a _]`). The search starts in the pseudo-attribute's own clause list and moves outwards, before each enclosing `not` / `or`; it crosses a `not-join` / `or-join` only for an entity variable in its join variables, and stops at a rule body, which never reads its caller's patterns. Terms are the same when they are one variable, one `$slot`, or constants naming one entity (`:alice` and its UUID). Before, they read the last pattern *the planner ran* that matched the entity, so the answer depended on the plan and differed under the `wasm` feature: in `[?e :person/name ?n] [?e :person/age ?a] [?e :db/tx-count ?t]`, `?t` could be the tx-count of either fact, and is now the `:person/age` fact's. Three further changes follow: (1) with no source, the query fails at parse time with the new `PRS-081` instead of matching every visible fact of the entity (`[?e :db/tx-count ?t]` before `[?e :person/name ?n]`, a `_` entity, a pseudo-attribute in a rule body whose only candidate is in the caller); to read every fact of an entity, write `[?e ?a _]` before it; (2) inside `not`, `not-join`, `or` and `or-join`, a pseudo-attribute reads the outer fact, so `[?e :person/name ?n] (not [?e :db/tx-count 1])` excludes the entities whose *name* fact is from tx 1, where before the body matched any fact of the entity; (3) rows that differ only in a source fact's metadata are no longer merged by `or`. `:db/valid-at` is unchanged. The pseudo-attribute pattern now binds or checks a value; it never scans facts, so the planner places it after its source rather than by selectivity.
- **A pseudo-attribute pattern never gives a variable its value** (#606). A variable bound by several patterns returns the value of the first real pattern in the `:where` text that binds it (#545); a `:db/valid-at` pattern that the planner ran first used to bind its entity variable itself, as a ref, so `[:find ?b ?t :where [?b :db/valid-at ?t] [?a :friend ?b] [?b :name "Bob"]]` returned the ref of `:bob` with source order and the keyword `:bob` stored by `:friend` natively. It returns the keyword now, whatever runs first. Only the keyword-or-ref kind changes; rows and counts are the same.
- **Every build reorders query clauses; the `wasm` feature does nothing** (#594, #596). The `wasm` Cargo feature kept patterns, `not` clauses and `or` branches in source order and turned off the `or` short-circuits (#250), for small browser datasets and a predictable order. With every answer independent of the plan (#594, #596, #606) it only made queries slower, and no first-party build enabled it (the browser package uses `browser`). The feature is kept, so builds that name it still resolve, but is deprecated and has no effect.

### Added

Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ cargo run < demos/demo_negation.txt
- `stratification.rs`: `DependencyGraph`, `stratify()` — negative edges + cycle detection
- `rules.rs`: `RuleRegistry` — thread-safe rule management
- `types.rs`: `EdnValue`, `Pattern`, `DatalogQuery`, `AsOf`, `ValidAt`, `WhereClause` (incl. `Not`, `NotJoin`); `PseudoAttr` enum, `AttributeSpec` wrapper
- `optimizer.rs`: Selectivity-based join reordering; disabled under `wasm` feature. `reorder()` gates every reordering site; tests turn it off per thread (`set_reorder`, `minigraf::__reorder` under `test-oracle`, #596)
- `optimizer.rs`: Selectivity-based join reordering on every build (the `wasm` feature is a deprecated no-op). `reorder()` gates every reordering site; tests turn it off per thread (`set_reorder`, `minigraf::__reorder` under `test-oracle`, #596)
- `prepared.rs`: `BindValue`, `PreparedQuery` — parse-once/execute-many with named `$slot` bind slots; `prepare_query`, `substitute`
- `magic_sets.rs`: Magic-sets rewriting for demand-driven recursive rule evaluation (not applied to rules with `not`/`not-join`)

Expand Down
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ harness = false

[features]
default = []
# Deprecated, does nothing since v3.0.0: every build reorders query clauses
# (#594, #596). Kept so that builds naming it still resolve.
wasm = []
# Run the existing executor beside the streaming engine and compare every
# answer (#473 spec §9). Tests only.
Expand Down
2 changes: 1 addition & 1 deletion docs/TEST_COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ This covers the unreleased file-format-v8 work (#287, #371): the index-key value
- **Reliability and concurrency**: concurrent reads and writes, rollback behavior, same-process handle exclusion, cross-process locking, PID namespaces, NFS and container lock behavior, SIGKILL recovery, and queries seeing each concurrent transaction whole and reading one state for all their patterns, including while a checkpoint runs (`tests/snapshot_atomicity_test.rs`, #554, #555, #558). Generation pins (#543): a pinned generation's pages are never written by later saves or index rebuilds and are reused once the pin is dropped (by page id, `storage::persistent_facts`, `storage::pins`, `storage::page`); fact logs stay correct while checkpoints, rebuilds and concurrent writers run, and see whole transactions only (`tests/fact_log_test.rs`). Query and prepared-query cursors keep their answer and their pages across writes, checkpoints and index rebuilds, and the file stops growing once they close (`tests/cursor_test.rs`); a snapshot leaves out facts appended after it on every scan (`graph::storage`); a crash at every write point of a save with a pinned generation loses no fact and no page, and the reopened database reuses the quarantined ids (`storage::persistent_facts`); the browser withholds free pages a pinned generation may read and releases them once out of force (`storage::pins::split_released`, and a `#[wasm_bindgen_test]` with an open `BrowserDb` cursor).
- **Streaming query engine** (#473, #544): every query the engine runs is also run by the existing executor under `cfg(test)` and the `test-oracle` feature, and the answers must agree (`query::engine::oracle`); CI runs the whole suite with the feature, so the model-based, property, golden-corpus and integration suites check the engine. `query::engine::tests` reaches every index choice, net-assert of pending records over committed ones, `:as-of`, valid time, long strings and reads of several batches, in memory, across checkpoints and after reopen, expression filters and bindings (including rebinding and failing expressions), pseudo-attribute columns, keyword/UUID/float constants against keywords, refs and UUID-named keywords, and checks which queries stay on the executor and that INT-026 and QRY-007 come first. Resumed `live_keys` reads equal one live read over random histories (`storage::persistent_facts`). Lazy cursors (`cursor::tests`, #544 PR 3): one pull reads one internal batch of the scan (operator counters), rows left in a batch need no pull, a closed stream reads nothing more, a filter's counters chain, and a cursor's answer is unchanged by writes and by checkpoints that free the leaves it has yet to read (file-backed) or by later writes (in memory); a read error after some rows are gathered comes on the next call, then the cursor ends (`query::engine::tests`). DICT lookups are cached for one pull only, and provisional ids keep their values across pulls (`query::engine::ids`). Joins (#545 PR 2): the oracle matrix adds two- and three-pattern joins on the entity (`IndexNestedLoopJoin`), value and attribute positions (`HashJoin`), cross products, keywords and refs of one entity in both text orders, UUID-named keywords, attributes that name entities, floats `0.0`/`-0.0`, long strings committed and pending, and expressions between joins, at every temporal; joins whose answers span many batches (one probe with more than a batch of matches, cross products, tables built on either side); the planner's join choice; a keyword's entity id (`query::engine::ids`). Pseudo-attributes across joins (#545 PR 3): the metadata of the last pattern before them that matched their entity, chosen per row (a pattern on another entity variable that equals theirs on some rows only, constant entities), pseudo-attribute patterns planned before any pattern with a stored attribute (joined with every fact of their entity, including cross products), and `:db/valid-at` at every temporal with variables, `nil` and integer constants; which pseudo-attribute shapes stay on the executor.
- **Prepared time travel** (#603): `tests/prepared_statements_test.rs` runs literal `:as-of` (counter and timestamp), `:valid-at`, `:any-valid-time` and a per-fact pseudo-attribute query through `execute`, a prepared `execute` and a prepared cursor, which must agree; a `prepared.rs` unit test checks that `substitute` keeps non-slot `:as-of` and `:valid-at`.
- **Join rules** (`tests/join_rules_test.rs`, 10 tests; #545): a join on a float value pairs `0.0` with `-0.0`, throu 8000 gh the hash key and as a shared variable, and a float constant still matches within epsilon; a variable bound to a keyword by one pattern and to the ref of the same entity by another returns the kind from the first pattern in `:where` text order, when the planner runs the patterns in another order, through the attribute position, before and after a rule call, and with a repeated pattern; an expression binding unifies with a pattern binding the same variable, and the first binder in the text, pattern or expression, gives the value (#596); a pseudo-attribute pattern never replaces a value (native plan order only: under `wasm` the pattern runs first and binds the variable). Under the `test-oracle` feature the joins among them also run on the streaming engine. A unit test checks that `-0.0` and `0.0` hash alike.
- **Join rules** (`tests/join_rules_test.rs`, 10 tests; #545): a join on a float value pairs `0.0` with `-0.0`, through the hash key and as a shared variable, and a float constant still matches within epsilon; a variable bound to a keyword by one pattern and to the ref of the same entity by another returns the kind from the first pattern in `:where` text order, when the planner runs the patterns in another order, through the attribute position, before and after a rule call, and with a repeated pattern; an expression binding unifies with a pattern binding the same variable, and the first binder in the text, pattern or expression, gives the value (#596); a pseudo-attribute pattern never gives a variable its value, even when it runs first (#606; the plan-independence matrix checks it with reordering off). Under the `test-oracle` feature the joins among them also run on the streaming engine. A unit test checks that `-0.0` and `0.0` hash alike.
- **Pseudo-attribute sources** (`tests/pseudo_source_test.rs`, 12 tests; #594): a per-fact pseudo-attribute reads the nearest earlier pattern in the text with the same entity term (#594's example in both text orders), two pseudo-attributes share a source, `[:alice ?a _]` gives one row per fact, constant and bound value terms check, `:alice` and its UUID are one term while `?e` and `:alice` are not, PRS-081 for a pattern after it, alone, `_`/`?_x` entities, another entity's pattern and a rule call; a `not` body reads the outer fact (and a source inside the body), `not-join` crosses for a listed variable only, `or` branches read the outer fact and never a sibling, `or-join`, closed rule bodies, `$slot`s by name at prepare time, and `:db/valid-at` once per fact. Parser unit tests check the hidden variables (`?#<n>.<attr>`), the join variable a crossed `not-join` gains, and the PRS-081 text; an optimizer test places a pseudo-attribute pattern after its source and the pattern binding its value; matcher tests read the hidden variable, a substituted integer, and nothing without a source; the engine tests run the sourced shapes and check PRS-081 for the old fallback shapes. The plan-independence matrix gains eleven pseudo-attribute shapes.
- **Plan independence** (`tests/plan_independence_test.rs`, 7 tests, `test-oracle` feature only; #596): a matrix of queries (joins in both text orders, cross products, keyword and ref, floats, long strings, filter and binding expressions, recursive rules and rules with `not`, `or`/`or-join` including the #250 short-circuit shapes, placed and deferred `not`/`not-join`, aggregates, `:as-of`, `:valid-at`, `:any-valid-time`), in memory and on file after a checkpoint and a reopen, through `execute` and a cursor, gives the same bag of rows with clause reordering on and off (`minigraf::__reorder`); #596's example and the unification cases (an expression whose output is its input, two expressions on one variable, integer vs float, `-0.0` vs `0.0`). Under `test-oracle` the model-based and property tests also rerun every generated query with reordering off. Unit tests cover `apply_expr` (equal, different, integer vs float, NaN, `-0.0`/`0.0` with and without replacing, keyword vs ref both ways, a failing expression), `Binders::expression`, the switch (source order kept, per thread, restored on drop), and the engine planner's `Bind` modes and build side with the switch off; the engine's join matrix gains six unification shapes.
- **Public diagnostics**: structured parser, query, storage, WAL, API, and internal error codes; the error-code registry is checked against [`ERROR_REFERENCE.md`](ERROR_REFERENCE.md).
Expand Down
1 change: 0 additions & 1 deletion src/query/datalog/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,6 @@ impl StratifiedEvaluator {
})
.collect();

// WASM omission: small datasets + determinism — see optimizer::selectivity_score().
if crate::query::datalog::optimizer::reorder() {
not_clauses.sort_by_key(|body| {
crate::query::datalog::optimizer::clause_cost(&WhereClause::Not(
Expand Down
15 changes: 10 additions & 5 deletions src/query/datalog/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,8 @@ type Binding = std::collections::HashMap<String, Value>;
/// returned as they were bound. An expression binding is a binder like a
/// pattern: when the variable is already bound it keeps the rows where its
/// result is equal (#596), and gives the value when it is first in the text.
/// A pseudo-attribute pattern never replaces a value.
/// A pseudo-attribute pattern never gives a variable its value: a real
/// pattern that binds it replaces its value, wherever it is (#606).
pub(crate) struct Binders {
/// Each top-level pattern (rule calls as patterns) and its position in
/// `:where`; taken as the planner hands them out.
Expand Down Expand Up @@ -1532,11 +1533,17 @@ impl Binders {
/// Record the pattern `p`, run next, and return the variables whose
/// value it replaces.
pub(crate) fn pattern(&mut self, p: &Pattern) -> Vec<String> {
let real = matches!(p.attribute, AttributeSpec::Real(_));
let at = match self.text.iter().position(|(q, _)| q == p) {
Some(i) => self.text.swap_remove(i).1,
// A pseudo-attribute pattern never gives a variable its value: it
// ranks after every real pattern, whatever runs first (#606).
Some(i) if real => self.text.swap_remove(i).1,
Some(i) => {
self.text.swap_remove(i);
usize::MAX
}
None => usize::MAX,
};
let real = matches!(p.attribute, AttributeSpec::Real(_));
let attribute = match &p.attribute {
AttributeSpec::Real(a) => Some(a),
AttributeSpec::Pseudo(_) => None,
Expand Down Expand Up @@ -2086,7 +2093,6 @@ pub(crate) fn apply_or_clauses(
let mut b: Vec<&Vec<WhereClause>> = branches.iter().collect();
// Sort branches by cost ascending so cheaper branches evaluate first,
// maximizing the chance the short-circuit below fires early (#250).
// WASM omission: small datasets + determinism — see optimizer::selectivity_score().
if optimizer::reorder() {
b.sort_by_key(|br| optimizer::branch_cost(br));
}
Expand Down Expand Up @@ -2313,7 +2319,6 @@ pub(crate) fn apply_or_clauses(
let mut b: Vec<&Vec<WhereClause>> = branches.iter().collect();
// Sort branches by cost ascending so cheaper branches evaluate first,
// maximizing the chance the short-circuit below fires early (#250).
// WASM omission: small datasets + determinism — see optimizer::selectivity_score().
if optimizer::reorder() {
b.sort_by_key(|br| optimizer::branch_cost(br));
}
Expand Down
Loading
Loading
0