From 87f850a44edb6b25480aadac619cfb66c2ebe70c Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 2 Jul 2018 09:22:27 -0700 Subject: [PATCH 1/7] Part 1: Move `intern_set` into `edn` crate. It's not great to keep lifting functionality higher and higher up the crate hierarchy, but we really do want to intern while we parse. Eventually, I expect that we will split the `edn` crate into `types` and `parsing`, and the `types` crate can depend on a more efficient interning dependency. --- core/src/lib.rs | 1 - db/src/db.rs | 15 ++++++++------- db/src/tx.rs | 3 +-- {core => edn}/src/intern_set.rs | 2 +- edn/src/lib.rs | 4 ++++ src/conn.rs | 5 +++-- src/entity_builder.rs | 14 ++++++++------ 7 files changed, 25 insertions(+), 19 deletions(-) rename {core => edn}/src/intern_set.rs (97%) diff --git a/core/src/lib.rs b/core/src/lib.rs index af9c71fe..4282a0b6 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -365,7 +365,6 @@ impl HasSchema for Schema { } } -pub mod intern_set; pub mod counter; pub mod util; diff --git a/db/src/db.rs b/db/src/db.rs index 327ef4ce..c8dd0fa5 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -1119,20 +1119,21 @@ mod tests { use debug; use errors; use edn; + use edn::{ + InternSet, + }; + use edn::entities::{ + OpType, + TempId, + }; + use mentat_core::{ HasSchema, Keyword, KnownEntid, attribute, }; - use mentat_core::intern_set::{ - InternSet, - }; use mentat_core::util::Either::*; - use edn::entities::{ - OpType, - TempId, - }; use rusqlite; use std::collections::{ BTreeMap, diff --git a/db/src/tx.rs b/db/src/tx.rs index dc777c34..c4ce8321 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -66,6 +66,7 @@ use db::{ PartitionMapping, }; use edn::{ + InternSet, Keyword, }; use entids; @@ -101,8 +102,6 @@ use mentat_core::{ now, }; -use mentat_core::intern_set::InternSet; - use edn::entities as entmod; use edn::entities::{ AttributePlace, diff --git a/core/src/intern_set.rs b/edn/src/intern_set.rs similarity index 97% rename from core/src/intern_set.rs rename to edn/src/intern_set.rs index 524cebf9..c8c7418f 100644 --- a/core/src/intern_set.rs +++ b/edn/src/intern_set.rs @@ -41,7 +41,7 @@ impl InternSet where T: Eq + Hash { /// /// ``` /// use std::rc::Rc; - /// use mentat_core::intern_set::InternSet; + /// use edn::intern_set::InternSet; /// /// let mut s = InternSet::new(); /// diff --git a/edn/src/lib.rs b/edn/src/lib.rs index a2a1dbcc..f9c57c94 100644 --- a/edn/src/lib.rs +++ b/edn/src/lib.rs @@ -23,6 +23,10 @@ extern crate serde; extern crate serde_derive; pub mod entities; +pub mod intern_set; +pub use intern_set::{ + InternSet, +}; // Intentionally not pub. mod namespaceable_name; pub mod query; diff --git a/src/conn.rs b/src/conn.rs index 9e5ca8d6..9f9e3ddd 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -41,6 +41,9 @@ use rusqlite::{ }; use edn; +use edn::{ + InternSet, +}; use mentat_core::{ Attribute, @@ -55,8 +58,6 @@ use mentat_core::{ ValueType, }; -use mentat_core::intern_set::InternSet; - use mentat_db::cache::{ InProgressCacheTransactWatcher, InProgressSQLiteAttributeCache, diff --git a/src/entity_builder.rs b/src/entity_builder.rs index c273b8ca..fe2e52ed 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -53,6 +53,14 @@ // We probably need both, but this file provides the latter. Unfortunately, Entity -- the input to // the transactor -- is intimately tied to EDN and to spanned values. +use edn::{ + InternSet, +}; +use edn::entities::{ + OpType, + TempId, +}; + use mentat_core::{ HasSchema, KnownEntid, @@ -60,7 +68,6 @@ use mentat_core::{ TypedValue, }; -use mentat_core::intern_set::InternSet; use mentat_core::util::Either; use mentat_db::{ @@ -75,11 +82,6 @@ use mentat_db::internal_types::{ TypedValueOr, }; -use edn::entities::{ - OpType, - TempId, -}; - use conn::{ InProgress, }; From 02a163a10fbb88d81dcbe10b98714d55c4330747 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 2 Jul 2018 09:34:57 -0700 Subject: [PATCH 2/7] Part 2: Use `ValueRc` in `InternSet`. We haven't observed performance issues using `Arc` instead of `Rc`, and we want to be able to include things that are interned (including, soon, `TempId` instances) in errors coming out of the transactor. (And `Rc` isn't `Sync`, so it can't be included in errors directly.) --- db/src/internal_types.rs | 20 ++++++++++---------- db/src/tx.rs | 6 ++---- edn/src/intern_set.rs | 16 +++++++++------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 72dda98d..9e323d61 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -17,7 +17,6 @@ use std::collections::{ BTreeSet, HashMap, }; -use std::rc::Rc; use mentat_core::KnownEntid; @@ -27,6 +26,14 @@ use edn; use edn::{ SpannedValue, ValueAndSpan, + ValueRc, +}; +use edn::entities; +use edn::entities::{ + EntityPlace, + OpType, + TempId, + TxFunction, }; use errors; @@ -47,13 +54,6 @@ use types::{ TypedValue, ValueType, }; -use edn::entities; -use edn::entities::{ - EntityPlace, - OpType, - TempId, - TxFunction, -}; impl TransactableValue for ValueAndSpan { fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result { @@ -150,10 +150,10 @@ use self::Either::*; pub type KnownEntidOr = Either; pub type TypedValueOr = Either; -pub type TempIdHandle = Rc; +pub type TempIdHandle = ValueRc; pub type TempIdMap = HashMap; -pub type LookupRef = Rc; +pub type LookupRef = ValueRc; /// Internal representation of an entid on its way to resolution. We either have the simple case (a /// numeric entid), a lookup-ref that still needs to be resolved (an atomized [a v] pair), or a temp diff --git a/db/src/tx.rs b/db/src/tx.rs index c4ce8321..c00b6a95 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -56,9 +56,6 @@ use std::collections::{ use std::iter::{ once, }; -use std::rc::{ - Rc, -}; use db; use db::{ @@ -68,6 +65,7 @@ use db::{ use edn::{ InternSet, Keyword, + ValueRc, }; use entids; use errors; @@ -303,7 +301,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(self.lookup_refs.intern((lr_a, lr_typed_value))) } - fn intern_temp_id(&mut self, temp_id: TempId) -> Rc { + fn intern_temp_id(&mut self, temp_id: TempId) -> ValueRc { self.temp_ids.intern(temp_id) } diff --git a/edn/src/intern_set.rs b/edn/src/intern_set.rs index c8c7418f..acec137e 100644 --- a/edn/src/intern_set.rs +++ b/edn/src/intern_set.rs @@ -12,7 +12,10 @@ use std::collections::HashSet; use std::hash::Hash; -use std::rc::Rc; + +use ::{ + ValueRc, +}; /// An `InternSet` allows to "intern" some potentially large values, maintaining a single value /// instance owned by the `InternSet` and leaving consumers with lightweight ref-counted handles to @@ -23,7 +26,7 @@ use std::rc::Rc; /// See https://en.wikipedia.org/wiki/String_interning for discussion. #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct InternSet where T: Eq + Hash { - pub inner: HashSet>, + pub inner: HashSet>, } impl InternSet where T: Eq + Hash { @@ -40,13 +43,12 @@ impl InternSet where T: Eq + Hash { /// Intern a value, providing a ref-counted handle to the interned value. /// /// ``` - /// use std::rc::Rc; - /// use edn::intern_set::InternSet; + /// use edn::{InternSet, ValueRc}; /// /// let mut s = InternSet::new(); /// /// let one = "foo".to_string(); - /// let two = Rc::new("foo".to_string()); + /// let two = ValueRc::new("foo".to_string()); /// /// let out_one = s.intern(one); /// assert_eq!(out_one, two); @@ -57,8 +59,8 @@ impl InternSet where T: Eq + Hash { /// assert_eq!(1, s.inner.len()); /// // assert!(&out_one.ptr_eq(&out_two)); // Nightly-only. /// ``` - pub fn intern>>(&mut self, value: R) -> Rc { - let key: Rc = value.into(); + pub fn intern>>(&mut self, value: R) -> ValueRc { + let key: ValueRc = value.into(); if self.inner.insert(key.clone()) { key } else { From 106d6fae11c80f611363e1ebc2fea12c2f18d807 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 2 Jul 2018 10:12:13 -0700 Subject: [PATCH 3/7] Part 3: Implement `Deref` and `DerefMut` for `InternSet`. This pattern is generally how newtype wrappers (like `struct Foo(Bar)`) are implemented in Rust. --- db/src/tx.rs | 6 +++--- edn/src/intern_set.rs | 26 ++++++++++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/db/src/tx.rs b/db/src/tx.rs index c00b6a95..46dd41d7 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -619,7 +619,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { let (terms_with_temp_ids_and_lookup_refs, tempid_set, lookup_ref_set) = self.entities_into_terms_with_temp_ids_and_lookup_refs(entities)?; // Pipeline stage 2: resolve lookup refs -> terms with tempids. - let lookup_ref_avs: Vec<&(i64, TypedValue)> = lookup_ref_set.inner.iter().map(|rc| &**rc).collect(); + let lookup_ref_avs: Vec<&(i64, TypedValue)> = lookup_ref_set.iter().map(|rc| &**rc).collect(); let lookup_ref_map: AVMap = self.store.resolve_avs(&lookup_ref_avs[..])?; let terms_with_temp_ids = self.resolve_lookup_refs(&lookup_ref_map, terms_with_temp_ids_and_lookup_refs)?; @@ -700,8 +700,8 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } // Verify that every tempid we interned either resolved or has been allocated. - assert_eq!(tempids.len(), tempid_set.inner.len()); - for tempid in &tempid_set.inner { + assert_eq!(tempids.len(), tempid_set.len()); + for tempid in tempid_set.iter() { assert!(tempids.contains_key(&**tempid)); } diff --git a/edn/src/intern_set.rs b/edn/src/intern_set.rs index acec137e..65d4e704 100644 --- a/edn/src/intern_set.rs +++ b/edn/src/intern_set.rs @@ -12,6 +12,10 @@ use std::collections::HashSet; use std::hash::Hash; +use std::ops::{ + Deref, + DerefMut, +}; use ::{ ValueRc, @@ -26,7 +30,21 @@ use ::{ /// See https://en.wikipedia.org/wiki/String_interning for discussion. #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct InternSet where T: Eq + Hash { - pub inner: HashSet>, + inner: HashSet>, +} + +impl Deref for InternSet where T: Eq + Hash { + type Target = HashSet>; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl DerefMut for InternSet where T: Eq + Hash { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } } impl InternSet where T: Eq + Hash { @@ -36,10 +54,6 @@ impl InternSet where T: Eq + Hash { } } - pub fn len(&self) -> usize { - self.inner.len() - } - /// Intern a value, providing a ref-counted handle to the interned value. /// /// ``` @@ -56,7 +70,7 @@ impl InternSet where T: Eq + Hash { /// /// let out_two = s.intern(two); /// assert_eq!(out_one, out_two); - /// assert_eq!(1, s.inner.len()); + /// assert_eq!(1, s.len()); /// // assert!(&out_one.ptr_eq(&out_two)); // Nightly-only. /// ``` pub fn intern>>(&mut self, value: R) -> ValueRc { From 76507623ac76cfe0f8d6d4fee127d9f6a62fb0df Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 2 Jul 2018 10:33:43 -0700 Subject: [PATCH 4/7] Part 4: Prepare EDN `Entity` type for interning tempids during parsing. This is all part of moving the entity builder away from building term instances and toward building entity instances. One of the nice things that the existing term interface does is allow consumers to use lightweight reference counted tempid handles; I don't want to lose that, so we'll build it into the entity data structures directly. --- db/src/internal_types.rs | 8 ++++---- db/src/tx.rs | 17 ++++++----------- edn/src/edn.rustpeg | 2 +- edn/src/entities.rs | 8 ++++++-- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 9e323d61..782ff22d 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -72,7 +72,7 @@ impl TransactableValue for ValueAndSpan { bail!(DbErrorKind::InputError(errors::InputError::BadEntityPlace)) } }, - Text(v) => Ok(EntityPlace::TempId(TempId::External(v))), + Text(v) => Ok(EntityPlace::TempId(TempId::External(v).into())), List(ls) => { let mut it = ls.iter(); match (it.next().map(|x| &x.inner), it.next(), it.next(), it.next()) { @@ -107,7 +107,7 @@ impl TransactableValue for ValueAndSpan { } fn as_tempid(&self) -> Option { - self.inner.as_text().cloned().map(TempId::External) + self.inner.as_text().cloned().map(TempId::External).map(|v| v.into()) } } @@ -123,7 +123,7 @@ impl TransactableValue for TypedValue { match self { TypedValue::Ref(x) => Ok(EntityPlace::Entid(entities::EntidOrIdent::Entid(x))), TypedValue::Keyword(x) => Ok(EntityPlace::Entid(entities::EntidOrIdent::Ident((*x).clone()))), - TypedValue::String(x) => Ok(EntityPlace::TempId(TempId::External((*x).clone()))), + TypedValue::String(x) => Ok(EntityPlace::TempId(TempId::External((*x).clone()).into())), TypedValue::Boolean(_) | TypedValue::Long(_) | TypedValue::Double(_) | @@ -134,7 +134,7 @@ impl TransactableValue for TypedValue { fn as_tempid(&self) -> Option { match self { - &TypedValue::String(ref s) => Some(TempId::External((**s).clone())), + &TypedValue::String(ref s) => Some(TempId::External((**s).clone()).into()), _ => None, } } diff --git a/db/src/tx.rs b/db/src/tx.rs index 46dd41d7..a398b114 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -65,7 +65,6 @@ use db::{ use edn::{ InternSet, Keyword, - ValueRc, }; use entids; use errors; @@ -301,15 +300,11 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(self.lookup_refs.intern((lr_a, lr_typed_value))) } - fn intern_temp_id(&mut self, temp_id: TempId) -> ValueRc { - self.temp_ids.intern(temp_id) - } - /// Allocate private internal tempids reserved for Mentat. Internal tempids just need to be /// unique within one transaction; they should never escape a transaction. fn allocate_mentat_id(&mut self) -> entmod::EntityPlace { self.mentat_id_count += 1; - entmod::EntityPlace::TempId(TempId::Internal(self.mentat_id_count)) + entmod::EntityPlace::TempId(TempId::Internal(self.mentat_id_count).into()) } fn entity_e_into_term_e(&mut self, x: entmod::EntityPlace) -> Result> { @@ -323,7 +318,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { }, entmod::EntityPlace::TempId(e) => { - Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(e)))) + Ok(Either::Right(LookupRefOrTempId::TempId(self.temp_ids.intern(e)))) }, entmod::EntityPlace::LookupRef(ref lookup_ref) => { @@ -369,7 +364,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // that the given value is in the attribute's value set, or (in // limited cases) coerce the value into the attribute's value set. match v.as_tempid() { - Some(tempid) => Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(tempid)))), + Some(tempid) => Ok(Either::Right(LookupRefOrTempId::TempId(self.temp_ids.intern(tempid)))), None => { if let TypedValue::Ref(entid) = v.into_typed_value(&self.schema, ValueType::Ref)? { Ok(Either::Left(KnownEntid(entid))) @@ -385,7 +380,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(Either::Left(KnownEntid(self.entity_a_into_term_a(entid)?))), entmod::ValuePlace::TempId(tempid) => - Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(tempid)))), + Ok(Either::Right(LookupRefOrTempId::TempId(self.temp_ids.intern(tempid)))), entmod::ValuePlace::LookupRef(ref lookup_ref) => Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))), @@ -456,7 +451,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // limited cases) coerce the value into the attribute's value set. if attribute.value_type == ValueType::Ref { match v.as_tempid() { - Some(tempid) => Either::Right(LookupRefOrTempId::TempId(in_process.intern_temp_id(tempid))), + Some(tempid) => Either::Right(LookupRefOrTempId::TempId(in_process.temp_ids.intern(tempid))), None => v.into_typed_value(&self.schema, attribute.value_type).map(Either::Left)?, } } else { @@ -468,7 +463,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Either::Left(TypedValue::Ref(in_process.entity_a_into_term_a(entid)?)), entmod::ValuePlace::TempId(tempid) => - Either::Right(LookupRefOrTempId::TempId(in_process.intern_temp_id(tempid))), + Either::Right(LookupRefOrTempId::TempId(in_process.temp_ids.intern(tempid))), entmod::ValuePlace::LookupRef(ref lookup_ref) => { if attribute.value_type != ValueType::Ref { diff --git a/edn/src/edn.rustpeg b/edn/src/edn.rustpeg index 21926b1f..2a642bea 100644 --- a/edn/src/edn.rustpeg +++ b/edn/src/edn.rustpeg @@ -260,7 +260,7 @@ tx_function -> TxFunction = "(" __ n:$(symbol_name) __ ")" { TxFunction { op: PlainSymbol::plain(n) } } entity_place -> EntityPlace - = v:raw_text { EntityPlace::TempId(TempId::External(v)) } + = v:raw_text { EntityPlace::TempId(TempId::External(v).into()) } / v:entid { EntityPlace::Entid(v) } / v:lookup_ref { EntityPlace::LookupRef(v) } / v:tx_function { EntityPlace::TxFunction(v) } diff --git a/edn/src/entities.rs b/edn/src/entities.rs index 17610e20..1ddb4552 100644 --- a/edn/src/entities.rs +++ b/edn/src/entities.rs @@ -13,6 +13,10 @@ use std::collections::BTreeMap; use std::fmt; +use value_rc::{ + ValueRc, +}; + use symbols::{ Keyword, PlainSymbol, @@ -93,7 +97,7 @@ pub enum ValuePlace { Entid(EntidOrIdent), // We never know at parse-time whether a string is really a tempid, but we will often know when // building entities programmatically. - TempId(TempId), + TempId(ValueRc), LookupRef(LookupRef), TxFunction(TxFunction), Vector(Vec>), @@ -104,7 +108,7 @@ pub enum ValuePlace { #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum EntityPlace { Entid(EntidOrIdent), - TempId(TempId), + TempId(ValueRc), LookupRef(LookupRef), TxFunction(TxFunction), } From 1cb1847aa6c2e021ef43e0a7b1ea0c8f8e2a49f3 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Tue, 3 Jul 2018 12:45:02 -0700 Subject: [PATCH 5/7] Part 5: Make existing `TermBuilder` actually build `Entity` instances. There are a few tricky details to call out here. The first is the `TransactableValueMarker` trait. This is strictly a marker (like `Sized`, for example) to give some control over what types can be used as value types in `Entity` instances. This expression is needed due to the network of `Into` and `From` relations between the parts of valid `Entity` instances. This allows to drop the `IntoThing` work-around trait and use the established patterns. (Observe that `KnownEntid` makes this a little harder, due to the cross-crate consistency restrictions.) The second is that we can get rid `{add,retract}_kw`, since the network of relations expresses the coercions directly. The third is that this commit doesn't change the name `TermBuilder`, even though it is now building `Entity` instances. This is because there's _already_ an `EntityBuilder` which fixes the `EntityPlace`. It's not clear whether the existing entity building interface should be removed or whether both should be renamed. That can be follow-up. --- core/src/types.rs | 29 ++++++ edn/src/entities.rs | 110 ++++++++++++++++++++ ffi/src/lib.rs | 69 +++++++------ src/conn.rs | 4 +- src/entity_builder.rs | 226 +++++++++++------------------------------- src/store.rs | 14 +-- src/vocabulary.rs | 12 +-- 7 files changed, 246 insertions(+), 218 deletions(-) diff --git a/core/src/types.rs b/core/src/types.rs index 716f943f..992ddb38 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -47,6 +47,14 @@ use ::edn::{ ValueRc, }; +use ::edn::entities::{ + AttributePlace, + EntidOrIdent, + EntityPlace, + ValuePlace, + TransactableValueMarker, +}; + use values; /// Represents one entid in the entid space. @@ -73,6 +81,24 @@ impl From for TypedValue { } } +impl Into> for KnownEntid { + fn into(self) -> EntityPlace { + EntityPlace::Entid(EntidOrIdent::Entid(self.0)) + } +} + +impl Into for KnownEntid { + fn into(self) -> AttributePlace { + AttributePlace::Entid(EntidOrIdent::Entid(self.0)) + } +} + +impl Into> for KnownEntid { + fn into(self) -> ValuePlace { + ValuePlace::Entid(EntidOrIdent::Entid(self.0)) + } +} + /// The attribute of each Mentat assertion has a :db/valueType constraining the value to a /// particular set. Mentat recognizes the following :db/valueType values. #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] @@ -215,6 +241,9 @@ pub enum TypedValue { Uuid(Uuid), // It's only 128 bits, so this should be acceptable to clone. } +/// `TypedValue` is the value type for programmatic use in transaction builders. +impl TransactableValueMarker for TypedValue {} + /// The values bound in a query specification can be: /// /// * Vecs of structured values, for multi-valued component attributes or nested expressions. diff --git a/edn/src/entities.rs b/edn/src/entities.rs index 1ddb4552..c6c27704 100644 --- a/edn/src/entities.rs +++ b/edn/src/entities.rs @@ -22,6 +22,20 @@ use symbols::{ PlainSymbol, }; +use types::{ + ValueAndSpan, +}; + +/// `EntityPlace` and `ValuePlace` embed values, either directly (i.e., `ValuePlace::Atom`) or +/// indirectly (i.e., `EntityPlace::LookupRef`). In order to maintain the graph of `Into` and +/// `From` relations, we need to ensure that `{Value,Entity}Place` can't match as a potential value. +/// (If it does, the `impl Into for T` default conflicts.) This marker trait allows to mark +/// acceptable values, thereby removing `{Entity,Value}Place` from consideration. +pub trait TransactableValueMarker {} + +/// `ValueAndSpan` is the value type coming out of the entity parser. +impl TransactableValueMarker for ValueAndSpan {} + /// A tempid, either an external tempid given in a transaction (usually as an `Value::Text`), /// or an internal tempid allocated by Mentat itself. #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] @@ -54,6 +68,18 @@ pub enum EntidOrIdent { Ident(Keyword), } +impl From for EntidOrIdent { + fn from(v: i64) -> Self { + EntidOrIdent::Entid(v) + } +} + +impl From for EntidOrIdent { + fn from(v: Keyword) -> Self { + EntidOrIdent::Ident(v) + } +} + impl EntidOrIdent { pub fn unreversed(&self) -> Option { match self { @@ -105,6 +131,54 @@ pub enum ValuePlace { MapNotation(MapNotation), } +impl From for ValuePlace { + fn from(v: EntidOrIdent) -> Self { + ValuePlace::Entid(v) + } +} + +impl From for ValuePlace { + fn from(v: TempId) -> Self { + ValuePlace::TempId(v.into()) + } +} + +impl From> for ValuePlace { + fn from(v: ValueRc) -> Self { + ValuePlace::TempId(v) + } +} + +impl From> for ValuePlace { + fn from(v: LookupRef) -> Self { + ValuePlace::LookupRef(v) + } +} + +impl From for ValuePlace { + fn from(v: TxFunction) -> Self { + ValuePlace::TxFunction(v) + } +} + +impl From>> for ValuePlace { + fn from(v: Vec>) -> Self { + ValuePlace::Vector(v) + } +} + +impl From for ValuePlace { + fn from(v: V) -> Self { + ValuePlace::Atom(v) + } +} + +impl From> for ValuePlace { + fn from(v: MapNotation) -> Self { + ValuePlace::MapNotation(v) + } +} + #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum EntityPlace { Entid(EntidOrIdent), @@ -113,11 +187,47 @@ pub enum EntityPlace { TxFunction(TxFunction), } +impl> From for EntityPlace { + fn from(v: E) -> Self { + EntityPlace::Entid(v.into()) + } +} + +impl From for EntityPlace { + fn from(v: TempId) -> Self { + EntityPlace::TempId(v.into()) + } +} + +impl From> for EntityPlace { + fn from(v: ValueRc) -> Self { + EntityPlace::TempId(v) + } +} + +impl From> for EntityPlace { + fn from(v: LookupRef) -> Self { + EntityPlace::LookupRef(v) + } +} + +impl From for EntityPlace { + fn from(v: TxFunction) -> Self { + EntityPlace::TxFunction(v) + } +} + #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum AttributePlace { Entid(EntidOrIdent), } +impl> From for AttributePlace { + fn from(v: A) -> Self { + AttributePlace::Entid(v.into()) + } +} + #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum OpType { Add, diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index b08d7d38..98c45d8d 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -120,7 +120,6 @@ pub use mentat::entity_builder::{ BuildTerms, EntityBuilder, InProgressBuilder, - IntoThing, }; pub mod android; @@ -339,7 +338,7 @@ pub unsafe extern "C" fn in_progress_entity_builder_from_temp_id<'m>(in_progress pub unsafe extern "C" fn in_progress_entity_builder_from_entid<'m>(in_progress: *mut InProgress<'m, 'm>, entid: c_longlong) -> *mut EntityBuilder { assert_not_null!(in_progress); let in_progress = Box::from_raw(in_progress); - Box::into_raw(Box::new(in_progress.builder().describe(&KnownEntid(entid)))) + Box::into_raw(Box::new(in_progress.builder().describe(KnownEntid(entid)))) } /// Starts a new transaction and creates a builder using the transaction @@ -392,7 +391,7 @@ pub unsafe extern "C" fn store_entity_builder_from_entid<'a, 'c>(store: *mut Sto assert_not_null!(store); let store = &mut *store; let result = store.begin_transaction().and_then(|in_progress| { - Ok(in_progress.builder().describe(&KnownEntid(entid))) + Ok(in_progress.builder().describe(KnownEntid(entid))) }); translate_result(result, error) } @@ -418,7 +417,7 @@ pub unsafe extern "C" fn in_progress_builder_add_string<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = c_char_to_string(value).into(); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -441,7 +440,7 @@ pub unsafe extern "C" fn in_progress_builder_add_long<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Long(value); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -465,7 +464,7 @@ pub unsafe extern "C" fn in_progress_builder_add_ref<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Ref(value); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -490,7 +489,7 @@ pub unsafe extern "C" fn in_progress_builder_add_keyword<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = kw_from_string(c_char_to_string(value)).into(); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -514,7 +513,7 @@ pub unsafe extern "C" fn in_progress_builder_add_boolean<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -538,7 +537,7 @@ pub unsafe extern "C" fn in_progress_builder_add_double<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -562,7 +561,7 @@ pub unsafe extern "C" fn in_progress_builder_add_timestamp<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::instant(value); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -588,7 +587,7 @@ pub unsafe extern "C" fn in_progress_builder_add_uuid<'a, 'c>( let value = &*value; let value = Uuid::from_bytes(value).expect("valid uuid"); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.add(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -612,7 +611,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_string<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = c_char_to_string(value).into(); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -636,7 +635,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_long<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Long(value); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -660,7 +659,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_ref<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Ref(value); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } @@ -685,7 +684,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_keyword<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = kw_from_string(c_char_to_string(value)).into(); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -709,7 +708,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_boolean<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -733,7 +732,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_double<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -757,7 +756,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_timestamp<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::instant(value); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -785,7 +784,7 @@ pub unsafe extern "C" fn in_progress_builder_retract_uuid<'a, 'c>( let value = &*value; let value = Uuid::from_bytes(value).expect("valid uuid"); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(KnownEntid(entid), &kw, value), error); + translate_void_result(builder.retract(KnownEntid(entid), kw, value), error); } /// Transacts and commits all the assertions and retractions that have been performed @@ -842,7 +841,7 @@ pub unsafe extern "C" fn entity_builder_add_string<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = c_char_to_string(value).into(); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -865,7 +864,7 @@ pub unsafe extern "C" fn entity_builder_add_long<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Long(value); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -888,7 +887,7 @@ pub unsafe extern "C" fn entity_builder_add_ref<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Ref(value); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -911,7 +910,7 @@ pub unsafe extern "C" fn entity_builder_add_keyword<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = kw_from_string(c_char_to_string(value)).into(); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -934,7 +933,7 @@ pub unsafe extern "C" fn entity_builder_add_boolean<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -957,7 +956,7 @@ pub unsafe extern "C" fn entity_builder_add_double<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -980,7 +979,7 @@ pub unsafe extern "C" fn entity_builder_add_timestamp<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::instant(value); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to assert `value` for `kw` on entity `entid`. @@ -1005,7 +1004,7 @@ pub unsafe extern "C" fn entity_builder_add_uuid<'a, 'c>( let value = &*value; let value = Uuid::from_bytes(value).expect("valid uuid"); let value: TypedValue = value.into(); - translate_void_result(builder.add_kw(&kw, value), error); + translate_void_result(builder.add(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1028,7 +1027,7 @@ pub unsafe extern "C" fn entity_builder_retract_string<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = c_char_to_string(value).into(); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1051,7 +1050,7 @@ pub unsafe extern "C" fn entity_builder_retract_long<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Long(value); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1074,7 +1073,7 @@ pub unsafe extern "C" fn entity_builder_retract_ref<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::Ref(value); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1097,7 +1096,7 @@ pub unsafe extern "C" fn entity_builder_retract_keyword<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = kw_from_string(c_char_to_string(value)).into(); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1120,7 +1119,7 @@ pub unsafe extern "C" fn entity_builder_retract_boolean<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1143,7 +1142,7 @@ pub unsafe extern "C" fn entity_builder_retract_double<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1166,7 +1165,7 @@ pub unsafe extern "C" fn entity_builder_retract_timestamp<'a, 'c>( let builder = &mut *builder; let kw = kw_from_string(c_char_to_string(kw)); let value: TypedValue = TypedValue::instant(value); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Uses `builder` to retract `value` for `kw` on entity `entid`. @@ -1192,7 +1191,7 @@ pub unsafe extern "C" fn entity_builder_retract_uuid<'a, 'c>( let value = &*value; let value = Uuid::from_bytes(value).expect("valid uuid"); let value: TypedValue = value.into(); - translate_void_result(builder.retract_kw(&kw, value), error); + translate_void_result(builder.retract(kw, value), error); } /// Transacts all the assertions and retractions that have been performed diff --git a/src/conn.rs b/src/conn.rs index 9f9e3ddd..05caf746 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -402,8 +402,8 @@ impl<'a, 'c> InProgress<'a, 'c> { /// This exists so you can make your own. pub fn transact_builder(&mut self, builder: TermBuilder) -> Result { builder.build() - .and_then(|(terms, tempid_set)| { - self.transact_terms(terms, tempid_set) + .and_then(|(terms, _tempid_set)| { + self.transact_entities(terms) }) } diff --git a/src/entity_builder.rs b/src/entity_builder.rs index fe2e52ed..62fd43f3 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -50,73 +50,65 @@ // // The second is to expose a declarative, programmatic builder pattern for constructing entities. // -// We probably need both, but this file provides the latter. Unfortunately, Entity -- the input to -// the transactor -- is intimately tied to EDN and to spanned values. +// We probably need both, but this file provides the latter. use edn::{ InternSet, + ValueRc, }; use edn::entities::{ + AttributePlace, + Entity, + EntityPlace, OpType, TempId, + ValuePlace, }; use mentat_core::{ - HasSchema, - KnownEntid, - Keyword, TypedValue, }; -use mentat_core::util::Either; - use mentat_db::{ TxReport, }; -use mentat_db::internal_types::{ - KnownEntidOr, - TempIdHandle, - Term, - TermWithTempIds, - TypedValueOr, -}; - use conn::{ InProgress, }; use errors::{ - MentatError, Result, }; -pub type Terms = (Vec, InternSet); +pub type Terms = (Vec>, InternSet); pub struct TermBuilder { tempids: InternSet, - terms: Vec, + terms: Vec>, } pub struct EntityBuilder { builder: T, - entity: KnownEntidOr, + entity: EntityPlace, } pub trait BuildTerms where Self: Sized { - fn named_tempid(&mut self, name: String) -> TempIdHandle; + fn named_tempid(&mut self, name: String) -> ValueRc; fn describe_tempid(self, name: &str) -> EntityBuilder; - fn describe(self, entity: E) -> EntityBuilder where E: IntoThing>; - fn add(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing>; - fn retract(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing>; + fn describe(self, entity: E) -> EntityBuilder where E: Into>; + fn add(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into>; + fn retract(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into>; } impl BuildTerms for TermBuilder { - fn named_tempid(&mut self, name: String) -> TempIdHandle { + fn named_tempid(&mut self, name: String) -> ValueRc { self.tempids.intern(TempId::External(name)) } @@ -125,28 +117,26 @@ impl BuildTerms for TermBuilder { self.describe(e) } - fn describe(self, entity: E) -> EntityBuilder where E: IntoThing> { + fn describe(self, entity: E) -> EntityBuilder where E: Into> { EntityBuilder { builder: self, - entity: entity.into_thing(), + entity: entity.into(), } } - fn add(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { - let e = e.into_thing(); - let v = v.into_thing(); - self.terms.push(Term::AddOrRetract(OpType::Add, e, a.into(), v)); + fn add(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into> { + self.terms.push(Entity::AddOrRetract { op: OpType::Add, e: e.into(), a: a.into(), v: v.into() }); Ok(()) } - fn retract(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { - let e = e.into_thing(); - let v = v.into_thing(); - self.terms.push(Term::AddOrRetract(OpType::Retract, e, a.into(), v)); + fn retract(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into> { + self.terms.push(Entity::AddOrRetract { op: OpType::Retract, e: e.into(), a: a.into(), v: v.into() }); Ok(()) } } @@ -168,23 +158,25 @@ impl TermBuilder { } #[allow(dead_code)] - pub fn numbered_tempid(&mut self, id: i64) -> TempIdHandle { + pub fn numbered_tempid(&mut self, id: i64) -> ValueRc { self.tempids.intern(TempId::Internal(id)) } } impl EntityBuilder where T: BuildTerms { - pub fn finish(self) -> (T, KnownEntidOr) { + pub fn finish(self) -> (T, EntityPlace) { (self.builder, self.entity) } - pub fn add(&mut self, a: KnownEntid, v: V) -> Result<()> - where V: IntoThing> { + pub fn add(&mut self, a: A, v: V) -> Result<()> + where A: Into, + V: Into> { self.builder.add(self.entity.clone(), a, v) } - pub fn retract(&mut self, a: KnownEntid, v: V) -> Result<()> - where V: IntoThing> { + pub fn retract(&mut self, a: A, v: V) -> Result<()> + where A: Into, + V: Into> { self.builder.retract(self.entity.clone(), a, v) } } @@ -209,8 +201,8 @@ impl<'a, 'c> InProgressBuilder<'a, 'c> { let mut in_progress = self.in_progress; let result = self.builder .build() - .and_then(|(terms, tempid_set)| { - in_progress.transact_terms(terms, tempid_set) + .and_then(|(terms, _tempid_set)| { + in_progress.transact_entities(terms) }); (in_progress, result) } @@ -228,7 +220,7 @@ impl<'a, 'c> InProgressBuilder<'a, 'c> { } impl<'a, 'c> BuildTerms for InProgressBuilder<'a, 'c> { - fn named_tempid(&mut self, name: String) -> TempIdHandle { + fn named_tempid(&mut self, name: String) -> ValueRc { self.builder.named_tempid(name) } @@ -237,70 +229,29 @@ impl<'a, 'c> BuildTerms for InProgressBuilder<'a, 'c> { self.describe(e) } - fn describe(self, entity: E) -> EntityBuilder> where E: IntoThing> { + fn describe(self, entity: E) -> EntityBuilder> where E: Into> { EntityBuilder { builder: self, - entity: entity.into_thing(), + entity: entity.into(), } } - fn add(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { + fn add(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into> { self.builder.add(e, a, v) } - fn retract(&mut self, e: E, a: KnownEntid, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { + fn retract(&mut self, e: E, a: A, v: V) -> Result<()> + where E: Into>, + A: Into, + V: Into> { self.builder.retract(e, a, v) } } -impl<'a, 'c> InProgressBuilder<'a, 'c> { - pub fn add_kw(&mut self, e: E, a: &Keyword, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { - let (attribute, value) = self.extract_kw_value(a, v.into_thing())?; - self.add(e, attribute, value) - } - - pub fn retract_kw(&mut self, e: E, a: &Keyword, v: V) -> Result<()> - where E: IntoThing>, - V: IntoThing> { - let (attribute, value) = self.extract_kw_value(a, v.into_thing())?; - self.retract(e, attribute, value) - } - - fn extract_kw_value(&mut self, a: &Keyword, v: TypedValueOr) -> Result<(KnownEntid, TypedValueOr)> { - let attribute: KnownEntid; - if let Some((attr, aa)) = self.in_progress.attribute_for_ident(a) { - if let Either::Left(ref tv) = v { - let provided = tv.value_type(); - let expected = attr.value_type; - if provided != expected { - bail!(MentatError::ValueTypeMismatch(provided, expected)); - } - } - attribute = aa; - } else { - bail!(MentatError::UnknownAttribute(a.to_string())); - } - Ok((attribute, v)) - } -} - impl<'a, 'c> EntityBuilder> { - pub fn add_kw(&mut self, a: &Keyword, v: V) -> Result<()> - where V: IntoThing> { - self.builder.add_kw(self.entity.clone(), a, v) - } - - pub fn retract_kw(&mut self, a: &Keyword, v: V) -> Result<()> - where V: IntoThing> { - self.builder.retract_kw(self.entity.clone(), a, v) - } - /// Build the terms from this builder and transact them against the current /// `InProgress`. This method _always_ returns the `InProgress` -- failure doesn't /// imply an automatic rollback. @@ -315,69 +266,6 @@ impl<'a, 'c> EntityBuilder> { } } -// Can't implement Into for Rc. -pub trait IntoThing: Sized { - fn into_thing(self) -> T; -} - -pub trait FromThing { - fn from_thing(v: T) -> Self; -} - -impl FromThing for T { - fn from_thing(v: T) -> T { - v - } -} - -impl IntoThing for F where I: FromThing { - fn into_thing(self) -> I { - I::from_thing(self) - } -} - -impl<'a> FromThing<&'a TempIdHandle> for TypedValueOr { - fn from_thing(v: &'a TempIdHandle) -> Self { - Either::Right(v.clone()) - } -} - -impl FromThing for TypedValueOr { - fn from_thing(v: TempIdHandle) -> Self { - Either::Right(v) - } -} - -impl FromThing for TypedValueOr { - fn from_thing(v: TypedValue) -> Self { - Either::Left(v) - } -} - -impl FromThing for KnownEntidOr { - fn from_thing(v: TempIdHandle) -> Self { - Either::Right(v) - } -} - -impl<'a> FromThing<&'a KnownEntid> for KnownEntidOr { - fn from_thing(v: &'a KnownEntid) -> Self { - Either::Left(v.clone()) - } -} - -impl FromThing for KnownEntidOr { - fn from_thing(v: KnownEntid) -> Self { - Either::Left(v) - } -} - -impl FromThing for TypedValueOr { - fn from_thing(v: KnownEntid) -> Self { - Either::Left(v.into()) - } -} - #[cfg(test)] mod testing { extern crate mentat_db; @@ -386,6 +274,8 @@ mod testing { Conn, Entid, HasSchema, + KnownEntid, + MentatError, Queryable, TypedValue, TxReport, @@ -421,7 +311,7 @@ mod testing { let mut in_progress = conn.begin_transaction(&mut sqlite).expect("begun successfully"); // This should fail: unrecognized entid. - match in_progress.transact_terms(terms, tempids).expect_err("expected transact to fail") { + match in_progress.transact_entities(terms).expect_err("expected transact to fail") { MentatError::DbError(e) => { assert_eq!(e.kind(), mentat_db::DbErrorKind::UnrecognizedEntid(999)); }, @@ -456,7 +346,7 @@ mod testing { let e_x = builder.named_tempid("x".into()); let v_many_1 = TypedValue::typed_string("Some text"); let v_many_2 = TypedValue::typed_string("Other text"); - builder.add_kw(e_x.clone(), &kw!(:foo/many), v_many_1).expect("add succeeded"); + builder.add(e_x.clone(), kw!(:foo/many), v_many_1).expect("add succeeded"); builder.add(e_x.clone(), a_many, v_many_2).expect("add succeeded"); builder.commit().expect("commit succeeded"); } @@ -510,7 +400,7 @@ mod testing { assert_eq!(tempids.len(), 2); assert_eq!(terms.len(), 4); - report = in_progress.transact_terms(terms, tempids).expect("add succeeded"); + report = in_progress.transact_entities(terms).expect("add succeeded"); let x = report.tempids.get("x").expect("our tempid has an ID"); let y = report.tempids.get("y").expect("our tempid has an ID"); assert_eq!(in_progress.lookup_value_for_attribute(*y, &foo_ref).expect("lookup succeeded"), diff --git a/src/store.rs b/src/store.rs index 21bad12c..8f04d00c 100644 --- a/src/store.rs +++ b/src/store.rs @@ -607,12 +607,12 @@ mod tests { let name = format!("todo{}", i); let uuid = Uuid::new_v4(); let mut builder = in_progress.builder().describe_tempid(&name); - builder.add_kw(&kw!(:todo/uuid), TypedValue::Uuid(uuid)).expect("Expected added uuid"); + builder.add(kw!(:todo/uuid), TypedValue::Uuid(uuid)).expect("Expected added uuid"); changeset.insert(uuid_entid.clone()); - builder.add_kw(&kw!(:todo/name), TypedValue::typed_string(&name)).expect("Expected added name"); + builder.add(kw!(:todo/name), TypedValue::typed_string(&name)).expect("Expected added name"); changeset.insert(name_entid.clone()); if i % 2 == 0 { - builder.add_kw(&kw!(:todo/completion_date), TypedValue::current_instant()).expect("Expected added date"); + builder.add(kw!(:todo/completion_date), TypedValue::current_instant()).expect("Expected added date"); changeset.insert(date_entid.clone()); } let (ip, r) = builder.transact(); @@ -622,8 +622,8 @@ mod tests { in_progress = ip; } let mut builder = in_progress.builder().describe_tempid("Label"); - builder.add_kw(&kw!(:label/name), TypedValue::typed_string("Label 1")).expect("Expected added name"); - builder.add_kw(&kw!(:label/color), TypedValue::typed_string("blue")).expect("Expected added color"); + builder.add(kw!(:label/name), TypedValue::typed_string("Label 1")).expect("Expected added name"); + builder.add(kw!(:label/color), TypedValue::typed_string("blue")).expect("Expected added color"); builder.commit().expect("expect transaction to occur"); } @@ -678,8 +678,8 @@ mod tests { for i in 0..3 { let name = format!("label{}", i); let mut builder = in_progress.builder().describe_tempid(&name); - builder.add_kw(&kw!(:label/name), TypedValue::typed_string(&name)).expect("Expected added name"); - builder.add_kw(&kw!(:label/color), TypedValue::typed_string("blue")).expect("Expected added color"); + builder.add(kw!(:label/name), TypedValue::typed_string(&name)).expect("Expected added name"); + builder.add(kw!(:label/color), TypedValue::typed_string("blue")).expect("Expected added color"); let (ip, _) = builder.transact(); in_progress = ip; } diff --git a/src/vocabulary.rs b/src/vocabulary.rs index 7e622ee2..42674b5b 100644 --- a/src/vocabulary.rs +++ b/src/vocabulary.rs @@ -812,14 +812,14 @@ impl VocabularySource for SimpleVocabularySource { impl<'a, 'c> VocabularyMechanics for InProgress<'a, 'c> { /// Turn the vocabulary into datoms, transact them, and on success return the outcome. fn install_vocabulary(&mut self, definition: &Definition) -> Result { - let (terms, tempids) = definition.description(self)?; - self.transact_terms(terms, tempids)?; + let (terms, _tempids) = definition.description(self)?; + self.transact_entities(terms)?; Ok(VocabularyOutcome::Installed) } fn install_attributes_for<'definition>(&mut self, definition: &'definition Definition, attributes: Vec<&'definition (Keyword, Attribute)>) -> Result { - let (terms, tempids) = definition.description_for_attributes(&attributes, self, None)?; - self.transact_terms(terms, tempids)?; + let (terms, _tempids) = definition.description_for_attributes(&attributes, self, None)?; + self.transact_entities(terms)?; Ok(VocabularyOutcome::InstalledMissingAttributes) } @@ -834,8 +834,8 @@ impl<'a, 'c> VocabularyMechanics for InProgress<'a, 'c> { // TODO: don't do work for attributes that are unchanged. Here we rely on the transactor // to elide duplicate datoms. - let (terms, tempids) = definition.description_diff(self, &from_version)?; - self.transact_terms(terms, tempids)?; + let (terms, _tempids) = definition.description_diff(self, &from_version)?; + self.transact_entities(terms)?; definition.post(self, &from_version)?; Ok(VocabularyOutcome::Upgraded) From 06056a8468ac0836c677038390d9036e6ea2c29d Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Tue, 3 Jul 2018 13:18:02 -0700 Subject: [PATCH 6/7] Part 6: Lift `TxReport` to `core` crate. The `core` create didn't exist when the `db` was started, but this type is clearly part of the public interface of Mentat. --- core/src/lib.rs | 5 +++++ core/src/tx_report.rs | 38 ++++++++++++++++++++++++++++++++++++++ db/src/db.rs | 2 +- db/src/lib.rs | 1 - db/src/tx.rs | 2 +- db/src/types.rs | 17 ----------------- src/conn.rs | 2 +- src/entity_builder.rs | 7 ++----- src/lib.rs | 2 +- src/store.rs | 2 +- 10 files changed, 50 insertions(+), 28 deletions(-) create mode 100644 core/src/tx_report.rs diff --git a/core/src/lib.rs b/core/src/lib.rs index 4282a0b6..5e406554 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -60,9 +60,14 @@ pub use cache::{ /// Core types defining a Mentat knowledge base. mod types; +mod tx_report; mod value_type_set; mod sql_types; +pub use tx_report::{ + TxReport, +}; + pub use types::{ Binding, Entid, diff --git a/core/src/tx_report.rs b/core/src/tx_report.rs new file mode 100644 index 00000000..1b30278f --- /dev/null +++ b/core/src/tx_report.rs @@ -0,0 +1,38 @@ +// Copyright 2018 Mozilla +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. + +#![allow(dead_code)] + +use std::collections::{ + BTreeMap, +}; + +use ::{ + DateTime, + Entid, + Utc, +}; + +/// A transaction report summarizes an applied transaction. +#[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] +pub struct TxReport { + /// The transaction ID of the transaction. + pub tx_id: Entid, + + /// The timestamp when the transaction began to be committed. + pub tx_instant: DateTime, + + /// A map from string literal tempid to resolved or allocated entid. + /// + /// Every string literal tempid presented to the transactor either resolves via upsert to an + /// existing entid, or is allocated a new entid. (It is possible for multiple distinct string + /// literal tempids to all unify to a single freshly allocated entid.) + pub tempids: BTreeMap, +} diff --git a/db/src/db.rs b/db/src/db.rs index c8dd0fa5..8ad0cc60 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -1131,6 +1131,7 @@ mod tests { HasSchema, Keyword, KnownEntid, + TxReport, attribute, }; use mentat_core::util::Either::*; @@ -1142,7 +1143,6 @@ mod tests { Term, TermWithTempIds, }; - use types::TxReport; use tx::{ transact_terms, }; diff --git a/db/src/lib.rs b/db/src/lib.rs index c7cf40eb..bd8fe154 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -106,7 +106,6 @@ pub use types::{ DB, PartitionMap, TransactableValue, - TxReport, }; pub fn to_namespaced_keyword(s: &str) -> Result { diff --git a/db/src/tx.rs b/db/src/tx.rs index a398b114..4179fa24 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -94,6 +94,7 @@ use mentat_core::{ DateTime, KnownEntid, Schema, + TxReport, Utc, attribute, now, @@ -119,7 +120,6 @@ use types::{ Entid, PartitionMap, TransactableValue, - TxReport, TypedValue, ValueType, }; diff --git a/db/src/types.rs b/db/src/types.rs index 70ea4964..138dd66b 100644 --- a/db/src/types.rs +++ b/db/src/types.rs @@ -95,23 +95,6 @@ pub type AVMap<'a> = HashMap<&'a AVPair, Entid>; // represents a set of entids that are correspond to attributes pub type AttributeSet = BTreeSet; -/// A transaction report summarizes an applied transaction. -#[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] -pub struct TxReport { - /// The transaction ID of the transaction. - pub tx_id: Entid, - - /// The timestamp when the transaction began to be committed. - pub tx_instant: DateTime, - - /// A map from string literal tempid to resolved or allocated entid. - /// - /// Every string literal tempid presented to the transactor either resolves via upsert to an - /// existing entid, or is allocated a new entid. (It is possible for multiple distinct string - /// literal tempids to all unify to a single freshly allocated entid.) - pub tempids: BTreeMap, -} - /// The transactor is tied to `edn::ValueAndSpan` right now, but in the future we'd like to support /// `TypedValue` directly for programmatic use. `TransactableValue` encapsulates the interface /// value types (i.e., values in the value place) need to support to be transacted. diff --git a/src/conn.rs b/src/conn.rs index 05caf746..eb54dbbf 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -53,6 +53,7 @@ use mentat_core::{ Keyword, Schema, StructuredMap, + TxReport, TypedValue, ValueRc, ValueType, @@ -74,7 +75,6 @@ use mentat_db::{ TransactWatcher, TxObservationService, TxObserver, - TxReport, }; use mentat_db::internal_types::TermWithTempIds; diff --git a/src/entity_builder.rs b/src/entity_builder.rs index 62fd43f3..bfc9dde6 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -66,11 +66,8 @@ use edn::entities::{ }; use mentat_core::{ - TypedValue, -}; - -use mentat_db::{ TxReport, + TypedValue, }; use conn::{ @@ -277,8 +274,8 @@ mod testing { KnownEntid, MentatError, Queryable, - TypedValue, TxReport, + TypedValue, }; use super::*; diff --git a/src/lib.rs b/src/lib.rs index b8e83cd2..f1da47a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,6 +41,7 @@ pub use mentat_core::{ Keyword, Schema, Binding, + TxReport, TypedValue, Uuid, Utc, @@ -56,7 +57,6 @@ pub use mentat_db::{ DB_SCHEMA_CORE, AttributeSet, TxObserver, - TxReport, new_connection, }; diff --git a/src/store.rs b/src/store.rs index 8f04d00c..1cfce8bc 100644 --- a/src/store.rs +++ b/src/store.rs @@ -30,12 +30,12 @@ use mentat_core::{ Entid, Keyword, StructuredMap, + TxReport, TypedValue, ValueRc, }; use mentat_db::{ TxObserver, - TxReport, }; use mentat_tolstoy::Syncer; From eb1df31ac423eb7a1b4e586ee6a3aba4398d975f Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Tue, 3 Jul 2018 13:52:02 -0700 Subject: [PATCH 7/7] Part 7: Improve `TermBuilder` interface; expose lookup refs and tx functions. These are functions on `TermBuilder` itself to prevent mixing mutable and immutable references in the most natural style. That is, ``` builder.add(e, a, builder.lookup_ref(...)) ``` fails because `add` borrows `builder` mutably and `lookup_ref` borrows `builder` immutably. There's nothing here that requires a specific builder (since we're not interning lookup refs on the builder, like we are tempids) so we don't need an instance. --- src/entity_builder.rs | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/entity_builder.rs b/src/entity_builder.rs index bfc9dde6..9ad9d162 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -54,14 +54,17 @@ use edn::{ InternSet, + PlainSymbol, ValueRc, }; use edn::entities::{ AttributePlace, Entity, EntityPlace, + LookupRef, OpType, TempId, + TxFunction, ValuePlace, }; @@ -91,7 +94,7 @@ pub struct EntityBuilder { } pub trait BuildTerms where Self: Sized { - fn named_tempid(&mut self, name: String) -> ValueRc; + fn named_tempid(&mut self, name: I) -> ValueRc where I: Into; fn describe_tempid(self, name: &str) -> EntityBuilder; fn describe(self, entity: E) -> EntityBuilder where E: Into>; fn add(&mut self, e: E, a: A, v: V) -> Result<()> @@ -105,12 +108,12 @@ pub trait BuildTerms where Self: Sized { } impl BuildTerms for TermBuilder { - fn named_tempid(&mut self, name: String) -> ValueRc { - self.tempids.intern(TempId::External(name)) + fn named_tempid(&mut self, name: I) -> ValueRc where I: Into { + self.tempids.intern(TempId::External(name.into())) } fn describe_tempid(mut self, name: &str) -> EntityBuilder { - let e = self.named_tempid(name.into()); + let e = self.named_tempid(name); self.describe(e) } @@ -158,6 +161,16 @@ impl TermBuilder { pub fn numbered_tempid(&mut self, id: i64) -> ValueRc { self.tempids.intern(TempId::Internal(id)) } + + pub fn lookup_ref(a: A, v: V) -> LookupRef + where A: Into, + V: Into { + LookupRef { a: a.into(), v: v.into() } + } + + pub fn tx_function(op: &str) -> TxFunction { + TxFunction { op: PlainSymbol::plain(op) } + } } impl EntityBuilder where T: BuildTerms { @@ -217,12 +230,12 @@ impl<'a, 'c> InProgressBuilder<'a, 'c> { } impl<'a, 'c> BuildTerms for InProgressBuilder<'a, 'c> { - fn named_tempid(&mut self, name: String) -> ValueRc { + fn named_tempid(&mut self, name: I) -> ValueRc where I: Into { self.builder.named_tempid(name) } fn describe_tempid(mut self, name: &str) -> EntityBuilder> { - let e = self.builder.named_tempid(name.into()); + let e = self.builder.named_tempid(name.to_string()); self.describe(e) } @@ -288,7 +301,7 @@ mod testing { #[test] fn test_entity_builder_bogus_entids() { let mut builder = TermBuilder::new(); - let e = builder.named_tempid("x".into()); + let e = builder.named_tempid("x"); let a1 = fake_known_entid(37); // :db/doc let a2 = fake_known_entid(999); let v = TypedValue::typed_string("Some attribute"); @@ -340,7 +353,7 @@ mod testing { let a_many = in_progress.get_entid(&kw!(:foo/many)).expect(":foo/many"); let mut builder = in_progress.builder(); - let e_x = builder.named_tempid("x".into()); + let e_x = builder.named_tempid("x"); let v_many_1 = TypedValue::typed_string("Some text"); let v_many_2 = TypedValue::typed_string("Other text"); builder.add(e_x.clone(), kw!(:foo/many), v_many_1).expect("add succeeded"); @@ -378,8 +391,8 @@ mod testing { // Scoped borrow of in_progress. { let mut builder = TermBuilder::new(); - let e_x = builder.named_tempid("x".into()); - let e_y = builder.named_tempid("y".into()); + let e_x = builder.named_tempid("x"); + let e_y = builder.named_tempid("y"); let a_ref = in_progress.get_entid(&foo_ref).expect(":foo/ref"); let a_one = in_progress.get_entid(&foo_one).expect(":foo/one"); let a_many = in_progress.get_entid(&foo_many).expect(":foo/many");