From 5a2fc5344f1530455f4b24daba79a5af05aaf4a7 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 2 Jun 2026 20:23:47 +0530 Subject: [PATCH 1/2] fix(query): include asserted flag in selective_fact_fetch dedup key (#285) When a WriteTransaction commits both a retraction and a new assertion for the same (entity, attribute) they share a tx_count, causing the previous dedup key (entity, attribute, tx_count) to silently drop the assertion. Adding `asserted` to the key ensures both facts survive the dedup and net_asserted_facts can correctly compute the final state. Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/executor.rs | 13 ++++++++----- tests/retraction_test.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 23ff52492..963983bbc 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -374,7 +374,7 @@ impl DatalogExecutor { /// selective, but multi-pattern joins still need attribute candidates for patterns that do not /// bind an entity. If any pattern has neither a bound entity nor a bound attribute, or if the /// distinct lookup count exceeds `threshold`, returns `None` to use a full scan. Otherwise - /// returns `Some(facts)`, deduplicated by `(entity, attribute, tx_count)`. + /// returns `Some(facts)`, deduplicated by `(entity, attribute, tx_count, asserted)`. fn selective_fact_fetch(&self, patterns: &[Pattern], threshold: usize) -> Option> { use std::collections::HashSet; @@ -405,15 +405,18 @@ impl DatalogExecutor { return None; } - // Dedup key: (entity uuid, attribute string, tx_count) — avoids Value debug formatting. - let mut seen: HashSet<(uuid::Uuid, String, u64)> = HashSet::new(); + // Dedup key: (entity uuid, attribute string, tx_count, asserted) — avoids Value debug + // formatting. Including `asserted` ensures that a retraction and an assertion committed in + // the same WriteTransaction (same tx_count) are both retained, since they differ only by + // the `asserted` flag. + let mut seen: HashSet<(uuid::Uuid, String, u64, bool)> = HashSet::new(); let mut all_facts: Vec = Vec::new(); for uid in &entity_ids { match self.storage.get_facts_by_entity(uid) { Ok(facts) => { for fact in facts { - let key = (fact.entity, fact.attribute.clone(), fact.tx_count); + let key = (fact.entity, fact.attribute.clone(), fact.tx_count, fact.asserted); if seen.insert(key) { all_facts.push(fact); } @@ -427,7 +430,7 @@ impl DatalogExecutor { match self.storage.get_facts_by_attribute(attr) { Ok(facts) => { for fact in facts { - let key = (fact.entity, fact.attribute.clone(), fact.tx_count); + let key = (fact.entity, fact.attribute.clone(), fact.tx_count, fact.asserted); if seen.insert(key) { all_facts.push(fact); } diff --git a/tests/retraction_test.rs b/tests/retraction_test.rs index ad8508aff..55640051b 100644 --- a/tests/retraction_test.rs +++ b/tests/retraction_test.rs @@ -122,6 +122,36 @@ fn test_retraction_rule_as_of_before_sees_fact() { ); } +// ── Test 7: Atomic retract+transact in WriteTransaction (issue #285) ───────── +// When both a retraction and a new assertion share the same tx_count (because they +// are committed together), selective_fact_fetch must not drop the assertion due to +// dedup on (entity, attribute, tx_count). + +#[test] +fn test_write_transaction_retract_and_transact_same_attr_query_sees_new_value() { + let db = Minigraf::in_memory().unwrap(); + + db.execute("(transact [[:order-42 :fsm/state :awaiting-payment]])") + .unwrap(); + + let mut tx = db.begin_write().unwrap(); + tx.execute("(retract [[:order-42 :fsm/state :awaiting-payment]])") + .unwrap(); + tx.execute("(transact [[:order-42 :fsm/state :paid]])") + .unwrap(); + tx.commit().unwrap(); + + let result = db + .execute("(query [:find ?order ?state :where [?order :fsm/state ?state]])") + .unwrap(); + match result { + minigraf::QueryResult::QueryResults { results, .. } => { + assert_eq!(results.len(), 1, "expected exactly one result row"); + } + _ => panic!("expected QueryResults"), + } +} + // ── Test 6: Retraction + recursive rule — as-of after retraction ────────────── #[test] From 1f78fc227bfea19111888fdd5d1d33856ce60eff Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 2 Jun 2026 20:24:01 +0530 Subject: [PATCH 2/2] style: rustfmt key tuple in selective_fact_fetch Co-Authored-By: Claude Sonnet 4.6 --- src/query/datalog/executor.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/query/datalog/executor.rs b/src/query/datalog/executor.rs index 963983bbc..9c75045b0 100644 --- a/src/query/datalog/executor.rs +++ b/src/query/datalog/executor.rs @@ -416,7 +416,12 @@ impl DatalogExecutor { match self.storage.get_facts_by_entity(uid) { Ok(facts) => { for fact in facts { - let key = (fact.entity, fact.attribute.clone(), fact.tx_count, fact.asserted); + let key = ( + fact.entity, + fact.attribute.clone(), + fact.tx_count, + fact.asserted, + ); if seen.insert(key) { all_facts.push(fact); } @@ -430,7 +435,12 @@ impl DatalogExecutor { match self.storage.get_facts_by_attribute(attr) { Ok(facts) => { for fact in facts { - let key = (fact.entity, fact.attribute.clone(), fact.tx_count, fact.asserted); + let key = ( + fact.entity, + fact.attribute.clone(), + fact.tx_count, + fact.asserted, + ); if seen.insert(key) { all_facts.push(fact); }