(tx) Implement :db/retractEntity. (#378)

db/src/db.rs

@@ -549,6 +549,9 @@ pub trait MentatStoring {
     fn insert_non_fts_searches<'a>(&self, entities: &'a [ReducedEntity], search_type: SearchType) -> Result<()>;
     fn insert_fts_searches<'a>(&self, entities: &'a [ReducedEntity], search_type: SearchType) -> Result<()>;
 
+    // :db/retractEntity the given entids.
+    fn insert_retract_entities<'a>(&self, entids: &'a [Entid]) -> Result<()>;
+
     /// Finalize the underlying storage layer after a Mentat transaction.
     ///
     /// Use this to finalize temporary tables, complete indices, revert pragmas, etc, after the
@@ -982,6 +985,48 @@ impl MentatStoring for rusqlite::Connection {
         results.map(|_| ())
     }
 
+    /// Insert datoms corresponding to :db/retractEntity entities into the search results.
+    fn insert_retract_entities(&self, entids: &[Entid]) -> Result<()> {
+        let max_vars = self.limit(Limit::SQLITE_LIMIT_VARIABLE_NUMBER) as usize;
+        let bindings_per_statement = 2;
+
+        let chunks: itertools::IntoChunks<_> = entids.into_iter().chunks(max_vars / bindings_per_statement);
+
+        // We'd like to flat_map here, but it's not obvious how to flat_map across Result.
+        let results: Result<Vec<()>> = chunks.into_iter().map(|chunk| -> Result<()> {
+            let mut count = 0;
+            let mut params: Vec<&ToSql> = chunk.flat_map(|e| {
+                count += 1;
+                once(e as &ToSql)
+            }).collect();
+
+            // Two copies, first for testing e, then for testing v.
+            // TODO: perhaps we can reference the same named parameter multiple times, making this
+            // more efficient.
+            let mut params2 = params.clone();
+            params.append(&mut params2);
+
+            let values: String = repeat_values(count, 1);
+
+            // Note that the value for flags (-1) is nonsense.  Since these rows are being removed
+            // from datoms, flags/flags0 is not referenced and this is fine.  The value for the
+            // search type is also ignored.
+            let s: String = format!(r#"
+              INSERT INTO temp.search_results
+              SELECT d.e, d.a, d.v, d.value_type_tag, 0, -1, ':db.cardinality/many', d.rowid, d.v
+              FROM datoms AS d
+              WHERE d.e IN ({}) OR (d.value_type_tag = 0 AND d.v IN ({}))
+              "#, values, values);
+
+            let mut stmt = self.prepare_cached(s.as_str())?;
+            stmt.execute(&params)
+                .map(|_c| ())
+                .chain_err(|| "Could not retract entities!")
+        }).collect::<Result<Vec<()>>>();
+
+        results.and(Ok(()))
+    }
+
     fn commit_transaction(&self, tx_id: Entid) -> Result<()> {
         search(&self)?;
         insert_transaction(&self, tx_id)?;
@@ -2387,6 +2432,71 @@ mod tests {
                          Err("EDN value \'1.23\' is not the expected Mentat value type Ref"));
     }
 
+    #[test]
+    fn test_retract_entity() {
+        let mut conn = TestConn::default();
+
+        // Start by installing a few attributes.
+        assert_transact!(conn, "[[:db/add 111 :db/ident :test/many]
+                                 [:db/add 111 :db/valueType :db.type/long]
+                                 [:db/add 111 :db/cardinality :db.cardinality/many]
+                                 [:db/add 222 :db/ident :test/component]
+                                 [:db/add 222 :db/isComponent true]
+                                 [:db/add 222 :db/valueType :db.type/ref]
+                                 [:db/add 333 :db/ident :test/dangling]
+                                 [:db/add 333 :db/valueType :db.type/ref]]");
+
+        // Verify that we can retract simple entities by entid.
+        assert_transact!(conn, "[[:db/retractEntity 111]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[111 :db/ident :test/many ?tx false]
+                          [111 :db/valueType :db.type/long ?tx false]
+                          [111 :db/cardinality :db.cardinality/many ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // Verify that we can retract entities that don't exist.
+        assert_transact!(conn, "[[:db/retractEntity 111]]");
+
+        // Verify that we can retract simple entities by keyword.
+        assert_transact!(conn, "[[:db/retractEntity :test/component]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[222 :db/ident :test/component ?tx false]
+                          [222 :db/valueType :db.type/ref ?tx false]
+                          [222 :db/isComponent true ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // Some data for testing associated references.
+        assert_transact!(conn, "[[:db/add 555 :test/dangling 666]
+                                 [:db/add 666 :test/dangling 777]
+                                 [:db/add 777 :test/dangling 777]
+                                 [:db/add 888 :test/dangling 999]]");
+
+        // Verify that associated references are also retracted.
+        assert_transact!(conn, "[[:db/retractEntity 666]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[555 :test/dangling 666 ?tx false]
+                          [666 :test/dangling 777 ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // What happens if we have a self reference?
+        assert_transact!(conn, "[[:db/retractEntity 777]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[777 :test/dangling 777 ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // Verify that we can retract entities that aren't recognized, but that appear as dangling
+        // references.
+        assert_transact!(conn, "[[:db/retractEntity 999]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[888 :test/dangling 999 ?tx false]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+
+        // Let's make sure we actually retracted from the datoms table.
+        assert_matches!(conn.datoms(),
+                        "[[333 :db/ident :test/dangling]
+                          [333 :db/valueType :db.type/ref]]");
+    }
+
     #[test]
     fn test_cardinality_one_violation_existing_entity() {
         let mut conn = TestConn::default();

db/src/internal_types.rs

@@ -35,6 +35,7 @@ use mentat_tx::entities::{
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)]
 pub enum Term<E, V> {
     AddOrRetract(OpType, E, Entid, V),
+    RetractEntity(E),
 }
 
 use self::Either::*;
@@ -68,7 +69,9 @@ impl TermWithTempIds {
     pub(crate) fn unwrap(self) -> TermWithoutTempIds {
         match self {
             Term::AddOrRetract(op, Left(n), a, Left(v)) => Term::AddOrRetract(op, n, a, v),
-            _ => unreachable!(),
+            Term::AddOrRetract(_, _, _, _) => unreachable!(),
+            Term::RetractEntity(Left(e)) => Term::RetractEntity(e),
+            Term::RetractEntity(_) => unreachable!(),
         }
     }
 }

db/src/tx.rs

@@ -381,6 +381,11 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher {
                     }
                 },
 
+                Entity::RetractEntity(e) => {
+                    let e = in_process.entity_e_into_term_e(e.into())?;
+                    terms.push(Term::RetractEntity(e));
+                },
+
                 Entity::AddOrRetract { op, e, a, v } => {
                     if let Some(reversed_a) = a.unreversed() {
                         let reversed_e = in_process.entity_v_into_term_e(v, &a)?;
@@ -519,6 +524,10 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher {
                     let v = replace_lookup_ref(&lookup_ref_map, v, |x| TypedValue::Ref(x))?;
                     Ok(Term::AddOrRetract(op, e, a, v))
                 },
+                Term::RetractEntity(e) => {
+                    let e = replace_lookup_ref(&lookup_ref_map, e, |x| KnownEntid(x))?;
+                    Ok(Term::RetractEntity(e))
+                },
             }
         }).collect::<Result<Vec<_>>>()
     }
@@ -629,6 +638,9 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher {
         // Assertions that are :db.cardinality/many and :db.fulltext.
         let mut fts_many: Vec<db::ReducedEntity> = vec![];
 
+        // :db/retractEntity entities.
+        let mut retract_entities: Vec<Entid> = vec![];
+
         // We need to ensure that callers can't blindly transact entities that haven't been
         // allocated by this store.
 
@@ -677,6 +689,12 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher {
                         (true, true) => fts_many.push(reduced),
                     }
                 },
+
+                Term::RetractEntity(KnownEntid(e)) => {
+                    // TODO: Might update metadata?
+                    // TODO: Watching/counting datoms?
+                    retract_entities.push(e);
+                },
             }
         }
 
@@ -705,6 +723,8 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher {
             self.store.insert_fts_searches(&fts_many[..], db::SearchType::Exact)?;
         }
 
+        self.store.insert_retract_entities(&retract_entities[..])?;
+
         self.store.commit_transaction(self.tx_id)?;
         }
 

db/src/upsert_resolution.rs

@@ -124,6 +124,9 @@ impl Generation {
                 Term::AddOrRetract(op, Left(e), a, Left(v)) => {
                     inert.push(Term::AddOrRetract(op, Left(e), a, Left(v)));
                 },
+                t @ Term::RetractEntity(_) => {
+                    inert.push(t);
+                },
             }
         }
 
@@ -189,6 +192,7 @@ impl Generation {
                     }
                 },
                 Term::AddOrRetract(_, Left(_), _, Left(_)) => unreachable!(),
+                Term::RetractEntity(_) => unreachable!(), // This is a coding error -- these should not be in allocations.
             }
         }
 
@@ -233,6 +237,7 @@ impl Generation {
                     // [:db/retract ...] entities never allocate entids; they have to resolve due to
                     // other upserts (or they fail the transaction).
                 },
+                &Term::RetractEntity(_) => unreachable!(), // This is a coding error -- these should not be in allocations.
             }
         }
 
@@ -275,6 +280,7 @@ impl Generation {
                     }
                 },
                 Term::AddOrRetract(_, Left(_), _, Left(_)) => unreachable!(), // This is a coding error -- these should not be in allocations.
+                Term::RetractEntity(_) => unreachable!(), // This is a coding error -- these should not be in allocations.
             };
             populations.allocated.push(allocated);
         }

src/entity_builder.rs

@@ -111,6 +111,7 @@ pub trait BuildTerms where Self: Sized {
     fn retract<E, V>(&mut self, e: E, a: KnownEntid, v: V) -> Result<()>
     where E: IntoThing<KnownEntidOr<TempIdHandle>>,
           V: IntoThing<TypedValueOr<TempIdHandle>>;
+    fn retract_entity(&mut self, e: KnownEntid) -> Result<()>;
 }
 
 impl BuildTerms for TermBuilder {
@@ -147,6 +148,11 @@ impl BuildTerms for TermBuilder {
         self.terms.push(Term::AddOrRetract(OpType::Retract, e, a.into(), v));
         Ok(())
     }
+
+    fn retract_entity(&mut self, e: KnownEntid) -> Result<()> {
+        self.terms.push(Term::RetractEntity(Either::Left(e.into())));
+        Ok(())
+    }
 }
 
 impl TermBuilder {
@@ -253,6 +259,10 @@ impl<'a, 'c> BuildTerms for InProgressBuilder<'a, 'c> {
           V: IntoThing<TypedValueOr<TempIdHandle>> {
         self.builder.retract(e, a, v)
     }
+
+    fn retract_entity(&mut self, e: KnownEntid) -> Result<()> {
+        self.builder.retract_entity(e)
+    }
 }
 
 impl<'a, 'c> InProgressBuilder<'a, 'c> {

tx-parser/src/lib.rs

@@ -34,6 +34,7 @@ use combine::{
 use mentat_tx::entities::{
     AtomOrLookupRefOrVectorOrMapNotation,
     Entid,
+    EntidOrLookupRef,
     EntidOrLookupRefOrTempId,
     Entity,
     LookupRef,
@@ -88,6 +89,11 @@ def_parser!(Tx, lookup_ref, LookupRef, {
             .map(|(a, v)| LookupRef { a: a, v: v.clone().without_spans() }))
 });
 
+def_parser!(Tx, entid_or_lookup_ref, EntidOrLookupRef, {
+    Tx::entid().map(EntidOrLookupRef::Entid)
+        .or(Tx::lookup_ref().map(EntidOrLookupRef::LookupRef))
+});
+
 def_parser!(Tx, entid_or_lookup_ref_or_temp_id, EntidOrLookupRefOrTempId, {
     Tx::db_tx().map(EntidOrLookupRefOrTempId::TempId)
         .or(Tx::entid().map(EntidOrLookupRefOrTempId::Entid))
@@ -127,8 +133,7 @@ def_matches_namespaced_keyword!(Tx, literal_db_add, "db", "add");
 def_matches_namespaced_keyword!(Tx, literal_db_retract, "db", "retract");
 
 def_parser!(Tx, add_or_retract, Entity, {
-    vector().of_exactly(
-        // TODO: This commits as soon as it sees :db/{add,retract}, but could use an improved error message.
+    try(vector().of_exactly(
         (Tx::literal_db_add().map(|_| OpType::Add).or(Tx::literal_db_retract().map(|_| OpType::Retract)),
           try((Tx::entid_or_lookup_ref_or_temp_id(),
                Tx::forward_entid(),
@@ -144,7 +149,14 @@ def_parser!(Tx, add_or_retract, Entity, {
                     a: a,
                     v: v,
                 }
-            }))
+            })))
+});
+
+def_matches_namespaced_keyword!(Tx, literal_db_retract_entity, "db", "retractEntity");
+
+def_parser!(Tx, retract_entity, Entity, {
+    try(vector().of_exactly(
+        (Tx::literal_db_retract_entity(), Tx::entid_or_lookup_ref()).map(|(_, e)| Entity::RetractEntity(e))))
 });
 
 def_parser!(Tx, map_notation, MapNotation, {
@@ -157,6 +169,7 @@ def_parser!(Tx, map_notation, MapNotation, {
 
 def_parser!(Tx, entity, Entity, {
     Tx::add_or_retract()
+        .or(Tx::retract_entity())
         .or(Tx::map_notation().map(Entity::MapNotation))
 });
 
@@ -278,6 +291,52 @@ mod tests {
                    }));
     }
 
+
+    #[test]
+    fn test_retract_entity() {
+        let input = Value::Vector(vec![kw("db", "retractEntity"),
+                                       Value::Integer(101)]);
+
+        let input = input.with_spans();
+        let stream = input.atom_stream();
+        let result = Tx::entity().parse(stream).map(|x| x.0);
+
+        assert_eq!(result,
+                   Ok(Entity::RetractEntity(EntidOrLookupRef::Entid(Entid::Entid(101)))));
+    }
+
+    #[test]
+    fn test_retract_entity_kw() {
+        let input = Value::Vector(vec![kw("db", "retractEntity"),
+                                       kw("known", "ident")]);
+
+        let input = input.with_spans();
+        let stream = input.atom_stream();
+        let result = Tx::entity().parse(stream).map(|x| x.0);
+
+        assert_eq!(result,
+                   Ok(Entity::RetractEntity(EntidOrLookupRef::Entid(Entid::Ident(NamespacedKeyword::new("known", "ident"))))));
+    }
+
+    #[test]
+    fn test_retract_entity_lookup_ref() {
+        let input = Value::Vector(vec![kw("db", "retractEntity"),
+                                       Value::List(vec![Value::PlainSymbol(PlainSymbol::new("lookup-ref")),
+                                                        kw("test", "a1"),
+                                                        Value::Text("v1".into())].into_iter().collect()),
+                                       ]);
+
+        let input = input.with_spans();
+        let stream = input.atom_stream();
+        let result = Tx::entity().parse(stream).map(|x| x.0);
+
+        assert_eq!(result,
+                   Ok(Entity::RetractEntity(EntidOrLookupRef::LookupRef(LookupRef {
+                           a: Entid::Ident(NamespacedKeyword::new("test", "a1")),
+                           v: Value::Text("v1".into()),
+                       }))));
+    }
+
     #[test]
     fn test_lookup_ref() {
         let input = Value::Vector(vec![kw("db", "add"),