8000 fix(or): join an or sharing no variable with earlier clauses as a cross product (#610, backport) by adityamukho · Pull Request #613 · 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- **Rule queries with a literal argument return every row** (#608). Magic-sets rewriting restricts a rule to the facts its callers demand, but it took each rule's bound arguments from the first call it saw and assumed every other call matched. A call that did not — the same rule called with free arguments elsewhere in the query, a call inside `or` or `not`, a call from a rule the query calls with free arguments, or a rule-body call whose argument nothing bound yet — either failed with `INT-022` (`(chain ?x :c)` with `(chain ?x ?y) :- (link ?x ?m) (chain ?m ?y)`) or silently returned too few rows (`(link ?x :c) (link ?p ?q)` returned only the `:c` edges for `?p ?q`). A query with both arguments literal next to another rule call also lost its rows. The demand on each rule is now the one every call site supplies, and a rule some call site reads in full is evaluated in full. Present since v1.2.0; also fixed on v3.
- **An `or` that shares no variable with the earlier clauses returns its cross product** (#610). Such an `or` — `[?x :edge ?z] (or [?p :edge :b] [?p :edge :c])` — returned no rows when no branch had `not` or `not-join`, because the incoming rows were replaced by the branch results. Each incoming row now joins every branch row, in queries and rule bodies. `or-join` still refuses a join variable no earlier clause binds (`INT-018`): it filters, it does not bind. Also fixed on v3.
- **A prepared query keeps a literal `:as-of`, `:valid-at` or `:any-valid-time`** (#603). Binding a prepared query's slots dropped any `:as-of` or `:valid-at` that was not itself a slot, so `PreparedQuery::execute` ran at the current transaction time and valid now: `(query [:find ?v :as-of 1 :where [:a :n ?v]])` prepared returned the current rows, not those at transaction 1, and a prepared per-fact pseudo-attribute query with `:any-valid-time` failed with `INT-026`. Slot forms (`:as-of $tx`, `:valid-at $t`) were not affected. Present since 2026-04-04 (v1.x and every v2.x release).

## v2.0.6 — 2026-10-14
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ Auto-migrates v1/v2/v3/v4/v5/v6 → v7 on open/checkpoint.

## Test Coverage

**1267 tests** (1258 passing, 9 ignored; unit + integration + doc).
**1273 tests** (1264 passing, 9 ignored; unit + integration + doc).
See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

**Testing conventions** — see the Testing Conventions section below before writing any tests.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit

```bash
cargo run # interactive Datalog REPL
cargo test # run 1267 tests
cargo test # run 1273 tests
cargo run < demos/demo_recursive.txt # recursive rules demo
```

Expand Down
6 changes: 4 additions & 2 deletions docs/TEST_COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

## v2.0.6 Coverage Summary

**Verified**: 2026-10-14 with `cargo test`
**Verified**: 2026-10-16 with `cargo test`

**Result**: 1,258 passing tests and 9 ignored tests (1,267 total), including the golden-file corpus (#391)
**Result**: 1,264 passing tests and 9 ignored tests (1,273 total), including the golden-file corpus (#391)

For v2.0.5, `tests/edge_cases_test.rs` checks that a file of 1 to 4,095 bytes is `STG-045`, unchanged and with no WAL, when it is a database cut below one page or foreign bytes, and opens as a new database that takes a write and reads it back after reopen when it is a prefix of the initial header or all zeros (#506).

Expand All @@ -16,6 +16,8 @@ For v2.0.5, torn-WAL-tail tests (#513) check that opening a WAL writer cuts off

`tests/magic_sets_test.rs` checks rule queries with a literal argument where some call site cannot carry the magic-sets demand (#608): a rule-body call with an argument nothing binds yet, the same rule called with free arguments, a call from a rule called with free arguments, calls inside `or` and `not`, a fully literal call, and a call bound only by a variable; `magic_sets.rs` unit tests check the adornment each shape solves to.

`tests/disjunction_test.rs` checks that an `or` sharing no variable with the earlier clauses joins as a cross product (#610), before or after them, through the seeded path (a branch with `not`) and in a rule body, keeps the earlier clauses' variables and fact metadata (a pseudo-attribute), and gives no rows when no branch matches.

Not counted above: the `BrowserDb` tests run only on wasm32 with `wasm-bindgen-test` in a browser. Two of them check that the IndexedDB flush returns every page a write dirtied, and that a dirty page that cannot be read is an error rather than left out.

The v2.0.6 suite covers the public database API, the Datalog engine, storage and recovery paths, and the v2.0.0 locking and structured-error-code changes, plus v2.0.1's regression test for rebuilding indexes on open after several checkpoints (#370). Since v2.0.1, `tests/point_query_history_test.rs` and new storage unit tests check that attribute-narrowed bound-entity lookups return exactly what a full scan returns, and that `net_asserted_facts` matches its previous implementation on randomized histories (#323). Attribute-scan tests check that the AEVT range covers exactly the queried attribute, including non-ASCII names such as `:丿` and prefix siblings such as `:ab` for `:a` (#381). Incremental B+tree rebuild tests check that checkpoints which copy untouched index leaves produce exactly the entries and file checksum a full rebuild would, including over repeated checkpoints, that single-fact checkpoints do not fragment the index, and that a cyclic leaf chain or damaged slot directory is rejected rather than copied (#315). Directory-sync unit tests check that creating the `.graph` file or the WAL, and deleting the WAL, fsync the parent directory after the file operation, and that reopening an existing file does not (#389). Disjunction tests check that `or-join` returns an empty result, not INT-031, when no rows reach it: on an empty database, after a pattern that matches nothing, after a `not` that removes every row, and in a rule body (#405). A stratification unit test builds the same two-rule negative cycle 64 times and checks that the INT-054 message names the same predicate pair every time (#410). Magic-sets tests check that a recursive rule with a literal start that recurses through another rule returns the same results as without the literal (#297). API tests check that `execute()` and `WriteTransaction::execute()` reject `$slot` bind tokens in every position with API-010 (#407). The REPL `run()` test now runs in a child process with null stdin, so it no longer hangs when the test runner's stdin is an open non-TTY stream (#415). A fault-injection test crashes `save()` at each of its 47 page writes and syncs and checks that every reopen finds exactly the old or the new checkpointed facts (#374). For v2.0.3, storage unit tests and two `edge_cases_test.rs` tests check that more than 65,535 uncheckpointed facts each resolve to their own fact through entity lookups and attribute scans, in memory and file-backed, before and after a checkpoint, and that the widened `FactRef` encodes to the same bytes as before (#445). A WAL test reopens a database next to a WAL whose entries are all checkpointed, writes again, and checks that `:as-of` history is unchanged before and after another reopen; a unit test checks that restoring the transaction counter never lowers it (#447). Since v2.0.3, `tests/golden_corpus_test.rs` opens a copy of each committed golden file in `tests/golden/` (v7 files written by minigraf 2.0.3: one checkpoint, several checkpoints, indexes rebuilt on open, a crash-left WAL, an already-checkpointed WAL, same-transaction multi-values) and checks its manifest's CRC32, `tx_count` and query results after open, after a checkpoint and reopen, and after one more write; a second test checks that every golden file has a manifest (#391). A recursive-rules test runs rule queries on a database with more live facts than `max_results` and checks that they answer, and that the limit still stops a rule that derives more facts than it allows (#528). A WAL test restores a WAL whose entries are all checkpointed and checks that an explicit `checkpoint()` removes it, keeps the fact, and that the next write does not reuse tx 1; the golden-corpus test now requires `v7_stale_wal` to have no WAL after a checkpoint too (#457). A WAL test checks that dropping a handle opened with `wal_checkpoint_threshold(usize::MAX)` leaves the `.graph` file byte for byte unchanged under `Full` and `Normal` sync, keeps the WAL, and that a reopen replays it (#476).
Expand Down
16 changes: 2 additions & 14 deletions src/query/datalog/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1977,20 +1977,8 @@ pub(crate) fn apply_or_clauses(
.filter(|v| !v.starts_with("__") && branch_var_names.contains(v.as_str()))
.collect();

if shared_vars.is_empty() {
// No shared user-visible variables between incoming bindings and branch
// results. This is semantically equivalent to a cross-join, which only
// makes sense when `bindings` carries no meaningful state yet (i.e. it
// is a single empty binding — the query start state — or all incoming
// variables are fact-metadata keys that the branch does not reference).
// In that case the cross-join degenerates to "replace with branch union",
// which is what the original seeded evaluation produced. Incoming bindings
// that carry user-visible variables but none matching any branch variable
// would be silently dropped here — that situation should not arise given
// that `or` must share at least one variable with the surrounding clause.
bindings = union_bindings;
continue;
}
// With no shared variable every key is empty, so each incoming
// row joins every branch row: a cross product (#610).

// Build HashMap: shared-key tuple → Vec<branch Binding>
let mut branch_map: HashMap<Vec<(String, Value)>, Vec<Binding>> = HashMap::new();
Expand Down
84 changes: 84 additions & 0 deletions tests/disjunction_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,3 +670,87 @@ fn test_rule_with_or_join_body_no_matches_returns_empty() {
.expect("rule with or-join and no colored entities must not error");
assert_eq!(result_count(&r), 0, "no colored entities");
}

fn edge_db() -> Minigraf {
let db = db();
db.execute(r#"(transact [[:a :edge :b] [:b :edge :c]])"#)
.unwrap();
db
}

/// #610: an `or` sharing no variable with the earlier clauses joins as a cross product.
#[test]
fn test_or_with_no_shared_variable_is_cross_product() {
let db = edge_db();
let r = db
.execute(r#"(query [:find ?x ?p :where [?x :edge ?z] (or [?p :edge :b] [?p :edge :c])])"#)
.unwrap();
assert_eq!(result_count(&r), 4, "?x in {{a, b}} times ?p in {{a, b}}");

// Clause order does not change the answer.
let r = db
.execute(r#"(query [:find ?x ?p :where (or [?p :edge :b] [?p :edge :c]) [?x :edge ?z]])"#)
.unwrap();
assert_eq!(result_count(&r), 4, "or first: same cross product");
}

/// #610: the cross product keeps the earlier clauses' variables.
#[test]
fn test_or_cross_product_keeps_incoming_variables() {
let db = edge_db();
let r = db
.execute(r#"(query [:find ?z ?p :where [?x :edge ?z] (or [?p :edge :c])])"#)
.unwrap();
assert_eq!(result_count(&r), 2, "two ?z values, one ?p value");
}

/// #610: an `or` whose branches match nothing still empties the answer.
#[test]
fn test_or_with_no_shared_variable_and_no_match_is_empty() {
let db = edge_db();
let r = db
.execute(r#"(query [:find ?x ?p :where [?x :edge ?z] (or [?p :edge :zz] [?p :edge :yy])])"#)
.unwrap();
assert_eq!(result_count(&r), 0, "no branch matches");
}

/// #6 9A41 10: the seeded path (a branch with `not`) gives the same cross product.
#[test]
fn test_or_with_not_branch_and_no_shared_variable_is_cross_product() {
let db = edge_db();
let r = db
.execute(
r#"(query [:find ?x ?p
:where [?x :edge ?z]
(or (and [?p :edge :b] (not [?p :edge :zz]))
[?p :edge :c])])"#,
)
.unwrap();
assert_eq!(result_count(&r), 4, "seeded path: cross product");
}

/// #610: the same `or` in a rule body.
#[test]
fn test_rule_with_or_and_no_shared_variable_is_cross_product() {
let db = edge_db();
db.execute(r#"(rule [(pair ?x ?p) [?x :edge ?z] (or [?p :edge :b] [?p :edge :c])])"#)
.unwrap();
let r = db
.execute(r#"(query [:find ?x ?p :where (pair ?x ?p)])"#)
.unwrap();
assert_eq!(result_count(&r), 4, "rule body: cross product");
}

/// #610: incoming rows carrying fact metadata (a pseudo-attribute) still join.
#[test]
fn test_or_cross_product_after_pseudo_attribute() {
let db = edge_db();
let r = db
.execute(
r#"(query [:find ?x ?t ?p :any-valid-time
:where [?x :edge ?z] [?x :db/tx-count ?t]
(or [?p :edge :c])])"#,
)
.unwrap();
assert_eq!(result_count(&r), 2, "two edges, one tx-count each, one ?p");
}
Loading
0