From c764531049208457fe6364e850fc659b2591a4ed Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 20 Oct 2026 21:38:16 +0530 Subject: [PATCH] fix(rules): a pseudo-attribute in a rule body reads the stored fact (#661) Backport of the v3 fix. RecursiveEvaluator copied the base facts into its working storage with transact, one fact per call, so a positive rule body's :db/valid-from, :db/valid-to, :db/tx-count and :db/tx-id read the copy's metadata (evaluation time, forever, new counters). The base facts are now loaded as stored, as the mixed-rule pass already did. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT --- CHANGELOG.md | 1 + CLAUDE.md | 2 +- README.md | 2 +- docs/TEST_COVERAGE.md | 2 +- src/query/datalog/evaluator.rs | 11 +++---- tests/recursive_rules_test.rs | 59 ++++++++++++++++++++++++++++++++++ 6 files changed, 67 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b235b274..016e8e971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A pseudo-attribute in a rule body reads the stored fact** (#661). In a rule without `not`, `not-join`, `or` or `or-join`, `:db/valid-from`, `:db/valid-to`, `:db/tx-count` and `:db/tx-id` read the rule evaluator's working copy of the fact: valid-from was the time the query ran, valid-to forever, and the tx-count and tx-id those of the copy. With `[:a :edge :b]` valid from 2020-01-01 to 2021-01-01, `(rule [(vf ?x ?t) [?x :edge ?y] [?x :db/valid-from ?t]])` returned the query's time for `:a`; it now returns 2020-01-01, as the same pattern in a query body does. Stored data was never affected. Present since v0.15.0. - **A fact with more than four elements is rejected** (#649). `transact` read a fact's 4th element as its optional valid-time map and ignored everything after it, so `[[:alice :person/age 1 {} 99 :junk]]` stored `[:alice :person/age 1]` and dropped the rest without an error; two facts run together by a misplaced bracket lost one. A fact of more than four elements now fails with the new `PRS-108`, numbered as in v3.0.0, and the transaction writes nothing. A fact of five or more elements whose 4th is not a map now gets `PRS-108` instead of `PRS-047`. Present since v0.4.0. ### Tests diff --git a/CLAUDE.md b/CLAUDE.md index 43cb5b56e..b40516316 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,7 +175,7 @@ Auto-migrates v1/v2/v3/v4/v5/v6 → v7 on open/checkpoint. ## Test Coverage -**1317 tests** (1308 passing, 9 ignored; unit + integration + doc). +**1318 tests** (1309 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. diff --git a/README.md b/README.md index 4e7930ff5..b4ebb1390 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit ```bash cargo run # interactive Datalog REPL -cargo test # run 1317 tests +cargo test # run 1318 tests cargo run < demos/demo_recursive.txt # recursive rules demo ``` diff --git a/docs/TEST_COVERAGE.md b/docs/TEST_COVERAGE.md index 4fdc50e50..b992fc79a 100644 --- a/docs/TEST_COVERAGE.md +++ b/docs/TEST_COVERAGE.md @@ -4,7 +4,7 @@ **Verified**: 2026-10-20 with `cargo test` -**Result**: 1,308 passing tests and 9 ignored tests (1,317 total), including the golden-file corpus (#391) +**Result**: 1,309 passing tests and 9 ignored tests (1,318 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). Rule-body tests in `tests/recursive_rules_test.rs` check that a `:db/valid-at` pattern in a rule definition, in its `not` and `not-join` bodies and in its `or` branches binds the query's valid time, on the semi-naive, stratified and magic-sets paths (#617). diff --git a/src/query/datalog/evaluator.rs b/src/query/datalog/evaluator.rs index 6c7f31bbf..dd28148e0 100644 --- a/src/query/datalog/evaluator.rs +++ b/src/query/datalog/evaluator.rs @@ -127,13 +127,10 @@ impl RecursiveEvaluator { // Create storage for derived facts let derived = FactStorage::new(); - // Add base facts to derived storage - for fact in &base_facts { - derived.transact( - vec![(fact.entity, fact.attribute.clone(), fact.value.clone())], - None, - )?; - } + // Add base facts to derived storage as stored: a pseudo-attribute in a + // rule body reads their valid time and transaction (#661). + derived.load_facts(base_facts.iter().cloned())?; + derived.restore_tx_counter()?; // Track facts we've seen (for delta computation) using BTreeSet with canonical encoding. let mut seen_facts: BTreeSet<(uuid::Uuid, String, Vec)> = base_facts diff --git a/tests/recursive_rules_test.rs b/tests/recursive_rules_test.rs index 2be4dc4ff..d43f0f182 100644 --- a/tests/recursive_rules_test.rs +++ b/tests/recursive_rules_test.rs @@ -621,3 +621,62 @@ fn test_rule_or_branch_valid_at_binds_query_valid_time() { "one row per entity" ); } + +/// #661: a per-fact pseudo-attribute in a rule body reads the stored fact's +/// metadata, as it does in a query body. Positive rules used to read the +/// evaluator's working copy: valid-from was the time the query ran, +/// valid-to forever, and tx-count / tx-id the copy's. +#[test] +fn test_pseudo_attributes_in_rule_bodies_read_the_stored_fact() { + let db = Minigraf::in_memory().unwrap(); + exec( + &db, + r#"(transact {:valid-from "2020-01-01T00:00:00Z" :valid-to "2021-01-01T00:00:00Z"} [[:a :edge :b]])"#, + ); + exec(&db, "(transact [[:b :edge :c]])"); + exec(&db, "(transact [[:c :edge :d]])"); + let direct = |attr: &str| { + let mut rows = result_rows(exec( + &db, + &format!( + "(query [:find ?x ?m :valid-at :any-valid-time :where [?x :edge ?y] [?x {attr} ?m]])" + ), + )); + rows.sort(); + rows + }; + for (i, attr) in [ + ":db/valid-from", + ":db/valid-to", + ":db/tx-count", + ":db/tx-id", + ] + .into_iter() + .enumerate() + { + // A positive rule, and a recursive one whose later iterations read + // the base facts again. + exec( + &db, + &format!("(rule [(m{i} ?x ?m) [?x :edge ?y] [?x {attr} ?m]])"), + ); + exec( + &db, + &format!("(rule [(r{i} ?x ?m) [?x :edge ?y] [?x {attr} ?m]])"), + ); + exec( + &db, + &format!("(rule [(r{i} ?x ?m) [?x :edge ?y] [?x {attr} ?m] (r{i} ?y ?n)])"), + ); + let expected = direct(attr); + assert_eq!(expected.len(), 3, "three :edge facts"); + for rule in [format!("m{i}"), format!("r{i}")] { + let mut rows = result_rows(exec( + &db, + &format!("(query [:find ?x ?m :valid-at :any-valid-time :where ({rule} ?x ?m)])"), + )); + rows.sort(); + assert!(rows == expected, "a rule body reads the stored metadata"); + } + } +}