diff --git a/db/src/bootstrap.rs b/db/src/bootstrap.rs index c299f11a..bc910d41 100644 --- a/db/src/bootstrap.rs +++ b/db/src/bootstrap.rs @@ -290,7 +290,7 @@ pub(crate) fn bootstrap_schema() -> Schema { Schema::from_ident_map_and_triples(ident_map, bootstrap_triples).unwrap() } -pub(crate) fn bootstrap_entities() -> Vec { +pub(crate) fn bootstrap_entities() -> Vec> { let bootstrap_assertions: Value = Value::Vector([ symbolic_schema_to_assertions(&V1_SYMBOLIC_SCHEMA).expect("symbolic schema"), idents_to_assertions(&V1_IDENTS[..]), @@ -299,6 +299,6 @@ pub(crate) fn bootstrap_entities() -> Vec { // Failure here is a coding error (since the inputs are fixed), not a runtime error. // TODO: represent these bootstrap data errors rather than just panicing. - let bootstrap_entities: Vec = edn::parse::entities(&bootstrap_assertions.to_string()).expect("bootstrap assertions"); + let bootstrap_entities: Vec> = edn::parse::entities(&bootstrap_assertions.to_string()).expect("bootstrap assertions"); return bootstrap_entities; } diff --git a/db/src/db.rs b/db/src/db.rs index 8c20301b..17b74938 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -2084,12 +2084,12 @@ mod tests { // We cannot resolve lookup refs that aren't :db/unique. assert_transact!(conn, "[[:db/add (lookup-ref :test/not_unique :test/keyword) :test/not_unique :test/keyword]]", - Err("not yet implemented: Cannot resolve (lookup-ref 333 :test/keyword) with attribute that is not :db/unique")); + Err("not yet implemented: Cannot resolve (lookup-ref 333 Keyword(Keyword(NamespaceableName { namespace: Some(\"test\"), name: \"keyword\" }))) with attribute that is not :db/unique")); // We type check the lookup ref's value against the lookup ref's attribute. assert_transact!(conn, "[[:db/add (lookup-ref :test/unique_value :test/not_a_string) :test/not_unique :test/keyword]]", - Err("EDN value \':test/not_a_string\' is not the expected Mentat value type String")); + Err("value \':test/not_a_string\' is not the expected Mentat value type String")); // Each lookup ref in the entity column must resolve assert_transact!(conn, @@ -2239,6 +2239,16 @@ mod tests { assert_matches!(tempids(&report), "{\"t\" 65537}"); + // Check that we can explode map notation with :db/id as a lookup-ref or tx-function. + let report = assert_transact!(conn, "[{:db/id (lookup-ref :db/ident :db/ident) :test/many 4} + {:db/id (transaction-tx) :test/many 5}]"); + assert_matches!(conn.last_transaction(), + "[[1 :test/many 4 ?tx true] + [?tx :db/txInstant ?ms ?tx true] + [?tx :test/many 5 ?tx true]]"); + assert_matches!(tempids(&report), + "{}"); + // Check that we can explode map notation with nested vector values. let report = assert_transact!(conn, "[{:test/many [1 2]}]"); assert_matches!(conn.last_transaction(), @@ -2434,7 +2444,7 @@ mod tests { // And here, a float. assert_transact!(conn, "[{:test/_dangling 1.23}]", - Err("EDN value \'1.23\' is not the expected Mentat value type Ref")); + Err("value \'1.23\' is not the expected Mentat value type Ref")); } #[test] diff --git a/db/src/errors.rs b/db/src/errors.rs index e2b31166..7dfca817 100644 --- a/db/src/errors.rs +++ b/db/src/errors.rs @@ -15,7 +15,6 @@ use std::collections::{ BTreeSet, }; -use edn; use rusqlite; use mentat_tx::entities::{ @@ -143,10 +142,10 @@ error_chain! { display("not yet implemented: {}", t) } - /// We've been given an EDN value that isn't the correct Mentat type. - BadEDNValuePair(value: edn::types::Value, value_type: ValueType) { - description("EDN value is not the expected Mentat value type") - display("EDN value '{}' is not the expected Mentat value type {:?}", value, value_type) + /// We've been given a value that isn't the correct Mentat type. + BadValuePair(value: String, value_type: ValueType) { + description("value is not the expected Mentat value type") + display("value '{}' is not the expected Mentat value type {:?}", value, value_type) } /// We've got corrupt data in the SQL store: a value and value_type_tag don't line up. diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index a402810f..306cb04a 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -43,64 +43,50 @@ use types::{ AVPair, Entid, Schema, + TransactableValue, TypedValue, ValueType, }; use mentat_tx::entities; use mentat_tx::entities::{ - EntidOrLookupRefOrTempId, + EntityPlace, OpType, TempId, TxFunction, }; -/// 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. -pub trait TransactableValue { - /// Coerce this value place into the given type. This is where we perform schema-aware - /// coercion, for example coercing an integral value into a ref where appropriate. - fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result; - - /// Make an entity place out of this value place. This is where we limit values in nested maps - /// to valid entity places. - fn into_entity_place(self) -> Result; - - fn as_tempid(&self) -> Option; -} - impl TransactableValue for ValueAndSpan { fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result { - schema.to_typed_value(&self.without_spans(), value_type) + schema.to_typed_value(&self, value_type) } - fn into_entity_place(self) -> Result { + fn into_entity_place(self) -> Result> { use self::SpannedValue::*; match self.inner { - Integer(v) => Ok(EntidOrLookupRefOrTempId::Entid(entities::Entid::Entid(v))), + Integer(v) => Ok(EntityPlace::Entid(entities::Entid::Entid(v))), Keyword(v) => { if v.is_namespaced() { - Ok(EntidOrLookupRefOrTempId::Entid(entities::Entid::Ident(v))) + Ok(EntityPlace::Entid(entities::Entid::Ident(v))) } else { // We only allow namespaced idents. bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)) } }, - Text(v) => Ok(EntidOrLookupRefOrTempId::TempId(TempId::External(v))), + Text(v) => Ok(EntityPlace::TempId(TempId::External(v))), List(ls) => { let mut it = ls.iter(); match (it.next().map(|x| &x.inner), it.next(), it.next(), it.next()) { // Like "(transaction-id)". (Some(&PlainSymbol(ref op)), None, None, None) => { - Ok(EntidOrLookupRefOrTempId::TxFunction(TxFunction { op: op.clone() })) + Ok(EntityPlace::TxFunction(TxFunction { op: op.clone() })) }, // Like "(lookup-ref)". (Some(&PlainSymbol(edn::PlainSymbol(ref s))), Some(a), Some(v), None) if s == "lookup-ref" => { match a.clone().into_entity_place()? { - EntidOrLookupRefOrTempId::Entid(a) => Ok(EntidOrLookupRefOrTempId::LookupRef(entities::LookupRef { a, v: v.clone().without_spans() })), - EntidOrLookupRefOrTempId::TempId(_) | - EntidOrLookupRefOrTempId::TxFunction(_) | - EntidOrLookupRefOrTempId::LookupRef(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + EntityPlace::Entid(a) => Ok(EntityPlace::LookupRef(entities::LookupRef { a: entities::AttributePlace::Entid(a), v: v.clone() })), + EntityPlace::TempId(_) | + EntityPlace::TxFunction(_) | + EntityPlace::LookupRef(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), } }, _ => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), @@ -125,6 +111,35 @@ impl TransactableValue for ValueAndSpan { } } +impl TransactableValue for TypedValue { + fn into_typed_value(self, _schema: &Schema, value_type: ValueType) -> Result { + if self.value_type() != value_type { + bail!(ErrorKind::BadValuePair(format!("{:?}", self), value_type)); + } + Ok(self) + } + + fn into_entity_place(self) -> Result> { + match self { + TypedValue::Ref(x) => Ok(EntityPlace::Entid(entities::Entid::Entid(x))), + TypedValue::Keyword(x) => Ok(EntityPlace::Entid(entities::Entid::Ident((*x).clone()))), + TypedValue::String(x) => Ok(EntityPlace::TempId(TempId::External((*x).clone()))), + TypedValue::Boolean(_) | + TypedValue::Long(_) | + TypedValue::Double(_) | + TypedValue::Instant(_) | + TypedValue::Uuid(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + } + } + + fn as_tempid(&self) -> Option { + match self { + &TypedValue::String(ref s) => Some(TempId::External((**s).clone())), + _ => None, + } + } +} + #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum Term { AddOrRetract(OpType, E, Entid, V), diff --git a/db/src/lib.rs b/db/src/lib.rs index 5e6af503..57be7490 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -97,6 +97,7 @@ pub use types::{ AttributeSet, DB, PartitionMap, + TransactableValue, TxReport, }; diff --git a/db/src/schema.rs b/db/src/schema.rs index 8ff2c19f..7f6d1c32 100644 --- a/db/src/schema.rs +++ b/db/src/schema.rs @@ -295,17 +295,17 @@ pub trait SchemaTypeChecking { /// /// Either assert that the given value is in the value type's value set, or (in limited cases) /// coerce the given value into the value type's value set. - fn to_typed_value(&self, value: &edn::Value, value_type: ValueType) -> Result; + fn to_typed_value(&self, value: &edn::ValueAndSpan, value_type: ValueType) -> Result; } impl SchemaTypeChecking for Schema { - fn to_typed_value(&self, value: &edn::Value, value_type: ValueType) -> Result { + fn to_typed_value(&self, value: &edn::ValueAndSpan, value_type: ValueType) -> Result { // TODO: encapsulate entid-ident-attribute for better error messages, perhaps by including // the attribute (rather than just the attribute's value type) into this function or a // wrapper function. - match TypedValue::from_edn_value(value) { + match TypedValue::from_edn_value(&value.clone().without_spans()) { // We don't recognize this EDN at all. Get out! - None => bail!(ErrorKind::BadEDNValuePair(value.clone(), value_type)), + None => bail!(ErrorKind::BadValuePair(format!("{}", value), value_type)), Some(typed_value) => match (value_type, typed_value) { // Most types don't coerce at all. (ValueType::Boolean, tv @ TypedValue::Boolean(_)) => Ok(tv), @@ -331,7 +331,7 @@ impl SchemaTypeChecking for Schema { (vt @ ValueType::Instant, _) | (vt @ ValueType::Keyword, _) | (vt @ ValueType::Ref, _) - => bail!(ErrorKind::BadEDNValuePair(value.clone(), vt)), + => bail!(ErrorKind::BadValuePair(format!("{}", value), vt)), } } } diff --git a/db/src/tx.rs b/db/src/tx.rs index 147b78c1..769f7d55 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -86,7 +86,6 @@ use internal_types::{ TermWithTempIds, TermWithTempIdsAndLookupRefs, TermWithoutTempIds, - TransactableValue, TypedValueOr, replace_lookup_ref, }; @@ -106,6 +105,7 @@ use mentat_core::intern_set::InternSet; use mentat_tx::entities as entmod; use mentat_tx::entities::{ + AttributePlace, Entity, OpType, TempId, @@ -114,17 +114,17 @@ use metadata; use rusqlite; use schema::{ SchemaBuilding, - SchemaTypeChecking, }; use tx_checking; use types::{ - Attribute, - AVPair, AVMap, + AVPair, + Attribute, Entid, PartitionMap, - TypedValue, + TransactableValue, TxReport, + TypedValue, ValueType, }; use upsert_resolution::{ @@ -166,17 +166,19 @@ pub struct Tx<'conn, 'a, W> where W: TransactWatcher { /// Remove any :db/id value from the given map notation, converting the returned value into /// something suitable for the entity position rather than something suitable for a value position. -pub fn remove_db_id(map: &mut entmod::MapNotation) -> Result> { +pub fn remove_db_id(map: &mut entmod::MapNotation) -> Result>> { // TODO: extract lazy defined constant. let db_id_key = entmod::Entid::Ident(Keyword::namespaced("db", "id")); - let db_id: Option = if let Some(id) = map.remove(&db_id_key) { + let db_id: Option> = if let Some(id) = map.remove(&db_id_key) { match id { - entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => Some(v.into_entity_place()?), - entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(_) | - entmod::AtomOrLookupRefOrVectorOrMapNotation::TxFunction(_) | - entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) | - entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) => { + entmod::ValuePlace::Entid(e) => Some(entmod::EntityPlace::Entid(e)), + entmod::ValuePlace::LookupRef(e) => Some(entmod::EntityPlace::LookupRef(e)), + entmod::ValuePlace::TempId(e) => Some(entmod::EntityPlace::TempId(e)), + entmod::ValuePlace::TxFunction(e) => Some(entmod::EntityPlace::TxFunction(e)), + entmod::ValuePlace::Atom(v) => Some(v.into_entity_place()?), + entmod::ValuePlace::Vector(_) | + entmod::ValuePlace::MapNotation(_) => { bail!(ErrorKind::InputError(errors::InputError::BadDbId)) }, } @@ -253,7 +255,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { /// /// The `Term` instances produce share interned TempId and LookupRef handles, and we return the /// interned handle sets so that consumers can ensure all handles are used appropriately. - fn entities_into_terms_with_temp_ids_and_lookup_refs(&self, entities: I) -> Result<(Vec, InternSet, InternSet)> where I: IntoIterator { + fn entities_into_terms_with_temp_ids_and_lookup_refs(&self, entities: I) -> Result<(Vec, InternSet, InternSet)> where I: IntoIterator> { struct InProcess<'a> { partition_map: &'a PartitionMap, schema: &'a Schema, @@ -287,18 +289,18 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { self.schema.require_entid(e) } - fn intern_lookup_ref(&mut self, lookup_ref: &entmod::LookupRef) -> Result { + fn intern_lookup_ref(&mut self, lookup_ref: &entmod::LookupRef) -> Result { let lr_a: i64 = match lookup_ref.a { - entmod::Entid::Entid(ref a) => *a, - entmod::Entid::Ident(ref a) => self.schema.require_entid(&a)?.into(), + AttributePlace::Entid(entmod::Entid::Entid(ref a)) => *a, + AttributePlace::Entid(entmod::Entid::Ident(ref a)) => self.schema.require_entid(&a)?.into(), }; let lr_attribute: &Attribute = self.schema.require_attribute_for_entid(lr_a)?; + let lr_typed_value: TypedValue = lookup_ref.v.clone().into_typed_value(&self.schema, lr_attribute.value_type)?; if lr_attribute.unique.is_none() { - bail!(ErrorKind::NotYetImplemented(format!("Cannot resolve (lookup-ref {} {}) with attribute that is not :db/unique", lr_a, lookup_ref.v))) + bail!(ErrorKind::NotYetImplemented(format!("Cannot resolve (lookup-ref {} {:?}) with attribute that is not :db/unique", lr_a, lr_typed_value))) } - let lr_typed_value: TypedValue = self.schema.to_typed_value(&lookup_ref.v, lr_attribute.value_type)?; Ok(self.lookup_refs.intern((lr_a, lr_typed_value))) } @@ -308,14 +310,14 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { /// 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::EntidOrLookupRefOrTempId { + fn allocate_mentat_id(&mut self) -> entmod::EntityPlace { self.mentat_id_count += 1; - entmod::EntidOrLookupRefOrTempId::TempId(TempId::Internal(self.mentat_id_count)) + entmod::EntityPlace::TempId(TempId::Internal(self.mentat_id_count)) } - fn entity_e_into_term_e(&mut self, x: entmod::EntidOrLookupRefOrTempId) -> Result> { + fn entity_e_into_term_e(&mut self, x: entmod::EntityPlace) -> Result> { match x { - entmod::EntidOrLookupRefOrTempId::Entid(e) => { + entmod::EntityPlace::Entid(e) => { let e = match e { entmod::Entid::Entid(ref e) => self.ensure_entid_exists(*e)?, entmod::Entid::Ident(ref e) => self.ensure_ident_exists(&e)?, @@ -323,15 +325,15 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(Either::Left(e)) }, - entmod::EntidOrLookupRefOrTempId::TempId(e) => { + entmod::EntityPlace::TempId(e) => { Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(e)))) }, - entmod::EntidOrLookupRefOrTempId::LookupRef(ref lookup_ref) => { + entmod::EntityPlace::LookupRef(ref lookup_ref) => { Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))) }, - entmod::EntidOrLookupRefOrTempId::TxFunction(ref tx_function) => { + entmod::EntityPlace::TxFunction(ref tx_function) => { match tx_function.op.0.as_str() { "transaction-tx" => Ok(Either::Left(self.tx_id)), unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), @@ -348,11 +350,11 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(a) } - fn entity_e_into_term_v(&mut self, x: entmod::EntidOrLookupRefOrTempId) -> Result> { + fn entity_e_into_term_v(&mut self, x: entmod::EntityPlace) -> Result> { self.entity_e_into_term_e(x).map(|r| r.map_left(|ke| TypedValue::Ref(ke.0))) } - fn entity_v_into_term_e(&mut self, x: entmod::AtomOrLookupRefOrVectorOrMapNotation, backward_a: &entmod::Entid) -> Result> { + fn entity_v_into_term_e(&mut self, x: entmod::ValuePlace, backward_a: &entmod::Entid) -> Result> { match backward_a.unreversed() { None => { bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for forward attribute"))); @@ -365,7 +367,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } match x { - entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { + entmod::ValuePlace::Atom(v) => { // Here is where we do schema-aware typechecking: we either assert // that the given value is in the attribute's value set, or (in // limited cases) coerce the value into the attribute's value set. @@ -382,20 +384,26 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(ref lookup_ref) => + entmod::ValuePlace::Entid(entid) => + 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)))), + + entmod::ValuePlace::LookupRef(ref lookup_ref) => Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))), - entmod::AtomOrLookupRefOrVectorOrMapNotation::TxFunction(ref tx_function) => { + entmod::ValuePlace::TxFunction(ref tx_function) => { match tx_function.op.0.as_str() { "transaction-tx" => Ok(Either::Left(KnownEntid(self.tx_id.0))), unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), } }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) => + entmod::ValuePlace::Vector(_) => bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", forward_a))), - entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) => + entmod::ValuePlace::MapNotation(_) => bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", forward_a))), } }, @@ -408,7 +416,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // We want to handle entities in the order they're given to us, while also "exploding" some // entities into many. We therefore push the initial entities onto the back of the deque, // take from the front of the deque, and explode onto the front as well. - let mut deque: VecDeque = VecDeque::default(); + let mut deque: VecDeque> = VecDeque::default(); deque.extend(entities); let mut terms: Vec = Vec::with_capacity(deque.len()); @@ -418,7 +426,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Entity::MapNotation(mut map_notation) => { // :db/id is optional; if it's not given, we generate a special internal tempid // to use for upserting. This tempid will not be reported in the TxReport. - let db_id: entmod::EntidOrLookupRefOrTempId = remove_db_id(&mut map_notation)?.unwrap_or_else(|| in_process.allocate_mentat_id()); + let db_id: entmod::EntityPlace = remove_db_id(&mut map_notation)?.unwrap_or_else(|| in_process.allocate_mentat_id()); // We're not nested, so :db/isComponent is not relevant. We just explode the // map notation. @@ -426,13 +434,15 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { deque.push_front(Entity::AddOrRetract { op: OpType::Add, e: db_id.clone(), - a: a, + a: AttributePlace::Entid(a), v: v, }); } }, Entity::AddOrRetract { op, e, a, v } => { + let AttributePlace::Entid(a) = a; + if let Some(reversed_a) = a.unreversed() { let reversed_e = in_process.entity_v_into_term_e(v, &a)?; let reversed_a = in_process.entity_a_into_term_a(reversed_a)?; @@ -443,7 +453,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { let attribute = self.schema.require_attribute_for_entid(a)?; let v = match v { - entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { + entmod::ValuePlace::Atom(v) => { // Here is where we do schema-aware typechecking: we either assert // that the given value is in the attribute's value set, or (in // limited cases) coerce the value into the attribute's value set. @@ -457,7 +467,13 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(ref lookup_ref) => { + entmod::ValuePlace::Entid(entid) => + 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))), + + entmod::ValuePlace::LookupRef(ref lookup_ref) => { if attribute.value_type != ValueType::Ref { bail!(ErrorKind::NotYetImplemented(format!("Cannot resolve value lookup ref for attribute {} that is not :db/valueType :db.type/ref", a))) } @@ -465,7 +481,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Either::Right(LookupRefOrTempId::LookupRef(in_process.intern_lookup_ref(lookup_ref)?)) }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::TxFunction(ref tx_function) => { + entmod::ValuePlace::TxFunction(ref tx_function) => { let typed_value = match tx_function.op.0.as_str() { "transaction-tx" => TypedValue::Ref(self.tx_id), unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), @@ -485,7 +501,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Either::Left(typed_value) }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(vs) => { + entmod::ValuePlace::Vector(vs) => { if !attribute.multival { bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value for attribute {} that is not :db.cardinality :db.cardinality/many", a))); } @@ -494,14 +510,14 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { deque.push_front(Entity::AddOrRetract { op: op.clone(), e: e.clone(), - a: entmod::Entid::Entid(a), + a: AttributePlace::Entid(entmod::Entid::Entid(a)), v: vv, }); } continue }, - entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(mut map_notation) => { + entmod::ValuePlace::MapNotation(mut map_notation) => { // TODO: consider handling this at the tx-parser level. That would be // more strict and expressive, but it would lead to splitting // AddOrRetract, which proliferates types and code, or only handling @@ -516,9 +532,9 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // :db/id is optional; if it's not given, we generate a special internal tempid // to use for upserting. This tempid will not be reported in the TxReport. - let db_id: Option = remove_db_id(&mut map_notation)?; + let db_id: Option> = remove_db_id(&mut map_notation)?; let mut dangling = db_id.is_none(); - let db_id: entmod::EntidOrLookupRefOrTempId = db_id.unwrap_or_else(|| in_process.allocate_mentat_id()); + let db_id: entmod::EntityPlace = db_id.unwrap_or_else(|| in_process.allocate_mentat_id()); // We're nested, so we want to ensure we're not creating "dangling" // entities that can't be reached. If we're :db/isComponent, then this @@ -557,7 +573,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { deque.push_front(Entity::AddOrRetract { op: OpType::Add, e: db_id.clone(), - a: entmod::Entid::Entid(inner_a), + a: AttributePlace::Entid(entmod::Entid::Entid(inner_a)), v: inner_v, }); } @@ -600,8 +616,8 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { /// /// This approach is explained in https://github.com/mozilla/mentat/wiki/Transacting. // TODO: move this to the transactor layer. - pub fn transact_entities(&mut self, entities: I) -> Result - where I: IntoIterator { + pub fn transact_entities(&mut self, entities: I) -> Result + where I: IntoIterator> { // Pipeline stage 1: entities -> terms with tempids and lookup refs. let (terms_with_temp_ids_and_lookup_refs, tempid_set, lookup_ref_set) = self.entities_into_terms_with_temp_ids_and_lookup_refs(entities)?; @@ -841,13 +857,14 @@ where W: TransactWatcher { /// /// This approach is explained in https://github.com/mozilla/mentat/wiki/Transacting. // TODO: move this to the transactor layer. -pub fn transact<'conn, 'a, I, W>(conn: &'conn rusqlite::Connection, +pub fn transact<'conn, 'a, I, V, W>(conn: &'conn rusqlite::Connection, partition_map: PartitionMap, schema_for_mutation: &'a Schema, schema: &'a Schema, watcher: W, entities: I) -> Result<(TxReport, PartitionMap, Option, W)> - where I: IntoIterator, + where I: IntoIterator>, + V: TransactableValue, W: TransactWatcher { let mut tx = start_tx(conn, partition_map, schema_for_mutation, schema, watcher)?; diff --git a/db/src/types.rs b/db/src/types.rs index 14f8b78e..af5c7f9f 100644 --- a/db/src/types.rs +++ b/db/src/types.rs @@ -29,6 +29,13 @@ pub use self::mentat_core::{ ValueType, }; +use mentat_tx::entities::{ + EntityPlace, + TempId, +}; + +use errors; + /// Represents one partition of the entid space. #[derive(Clone,Debug,Eq,Hash,Ord,PartialOrd,PartialEq)] pub struct Partition { @@ -104,3 +111,18 @@ pub struct TxReport { /// 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. +pub trait TransactableValue: Clone { + /// Coerce this value place into the given type. This is where we perform schema-aware + /// coercion, for example coercing an integral value into a ref where appropriate. + fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> errors::Result; + + /// Make an entity place out of this value place. This is where we limit values in nested maps + /// to valid entity places. + fn into_entity_place(self) -> errors::Result>; + + fn as_tempid(&self) -> Option; +} diff --git a/edn/src/edn.rustpeg b/edn/src/edn.rustpeg index 6689aab6..da0e9905 100644 --- a/edn/src/edn.rustpeg +++ b/edn/src/edn.rustpeg @@ -223,35 +223,35 @@ forward_entid -> Entid backward_entid -> Entid = v:raw_backward_keyword { Entid::Ident(v.to_reversed()) } -lookup_ref -> LookupRef - = "(" __ "lookup-ref" __ a:(entid) __ v:(value) __ ")" { LookupRef { a, v: v.without_spans() } } +lookup_ref -> LookupRef + = "(" __ "lookup-ref" __ a:(entid) __ v:(value) __ ")" { LookupRef { a: AttributePlace::Entid(a), v } } tx_function -> TxFunction = "(" __ n:$(symbol_name) __ ")" { TxFunction { op: PlainSymbol::plain(n) } } -entity_place -> EntidOrLookupRefOrTempId - = v:raw_text { EntidOrLookupRefOrTempId::TempId(TempId::External(v)) } - / v:entid { EntidOrLookupRefOrTempId::Entid(v) } - / v:lookup_ref { EntidOrLookupRefOrTempId::LookupRef(v) } - / v:tx_function { EntidOrLookupRefOrTempId::TxFunction(v) } +entity_place -> EntityPlace + = v:raw_text { EntityPlace::TempId(TempId::External(v)) } + / v:entid { EntityPlace::Entid(v) } + / v:lookup_ref { EntityPlace::LookupRef(v) } + / v:tx_function { EntityPlace::TxFunction(v) } -value_place_pair -> (Entid, AtomOrLookupRefOrVectorOrMapNotation) +value_place_pair -> (Entid, ValuePlace) = k:(entid) __ v:(value_place) { (k, v) } -map_notation -> MapNotation +map_notation -> MapNotation = "{" __ kvs:(value_place_pair*) __ "}" { kvs.into_iter().collect() } -value_place -> AtomOrLookupRefOrVectorOrMapNotation - = __ v:lookup_ref __ { AtomOrLookupRefOrVectorOrMapNotation::LookupRef(v) } - / __ v:tx_function __ { AtomOrLookupRefOrVectorOrMapNotation::TxFunction(v) } - / __ "[" __ vs:(value_place*) __ "]" __ { AtomOrLookupRefOrVectorOrMapNotation::Vector(vs) } - / __ v:map_notation __ { AtomOrLookupRefOrVectorOrMapNotation::MapNotation(v) } - / __ v:atom __ { AtomOrLookupRefOrVectorOrMapNotation::Atom(v) } +value_place -> ValuePlace + = __ v:lookup_ref __ { ValuePlace::LookupRef(v) } + / __ v:tx_function __ { ValuePlace::TxFunction(v) } + / __ "[" __ vs:(value_place*) __ "]" __ { ValuePlace::Vector(vs) } + / __ v:map_notation __ { ValuePlace::MapNotation(v) } + / __ v:atom __ { ValuePlace::Atom(v) } -pub entity -> Entity - = __ "[" __ op:(op) __ e:(entity_place) __ a:(forward_entid) __ v:(value_place) __ "]" __ { Entity::AddOrRetract { op, e: e, a, v: v } } - / __ "[" __ op:(op) __ e:(value_place) __ a:(backward_entid) __ v:(entity_place) __ "]" __ { Entity::AddOrRetract { op, e: v, a, v: e } } +pub entity -> Entity + = __ "[" __ op:(op) __ e:(entity_place) __ a:(forward_entid) __ v:(value_place) __ "]" __ { Entity::AddOrRetract { op, e: e, a: AttributePlace::Entid(a), v: v } } + / __ "[" __ op:(op) __ e:(value_place) __ a:(backward_entid) __ v:(entity_place) __ "]" __ { Entity::AddOrRetract { op, e: v, a: AttributePlace::Entid(a), v: e } } / __ map:map_notation __ { Entity::MapNotation(map) } -pub entities -> Vec +pub entities -> Vec> = __ "[" __ es:(entity*) __ "]" __ { es } diff --git a/edn/src/entities.rs b/edn/src/entities.rs index 7d22096d..eee3985a 100644 --- a/edn/src/entities.rs +++ b/edn/src/entities.rs @@ -17,10 +17,6 @@ use symbols::{ Keyword, PlainSymbol, }; -use types::{ - Value, - ValueAndSpan, -}; /// A tempid, either an external tempid given in a transaction (usually as an `Value::Text`), /// or an internal tempid allocated by Mentat itself. @@ -64,11 +60,11 @@ impl Entid { } #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] -pub struct LookupRef { - pub a: Entid, +pub struct LookupRef { + pub a: AttributePlace, // In theory we could allow nested lookup-refs. In practice this would require us to process // lookup-refs in multiple phases, like how we resolve tempids, which isn't worth the effort. - pub v: Value, // An atom. + pub v: V, // An atom. } /// A "transaction function" that exposes some value determined by the current transaction. The @@ -88,23 +84,34 @@ pub struct TxFunction { pub op: PlainSymbol, } -pub type MapNotation = BTreeMap; +pub type MapNotation = BTreeMap>; #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] -pub enum AtomOrLookupRefOrVectorOrMapNotation { - Atom(ValueAndSpan), - LookupRef(LookupRef), +pub enum ValuePlace { + // We never know at parse-time whether an integer or ident is really an entid, but we will often + // know when building entities programmatically. + Entid(Entid), + // We never know at parse-time whether a string is really a tempid, but we will often know when + // building entities programmatically. + TempId(TempId), + LookupRef(LookupRef), TxFunction(TxFunction), - Vector(Vec), - MapNotation(MapNotation), + Vector(Vec>), + Atom(V), + MapNotation(MapNotation), } #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] -pub enum EntidOrLookupRefOrTempId { +pub enum EntityPlace { Entid(Entid), - LookupRef(LookupRef), - TxFunction(TxFunction), TempId(TempId), + LookupRef(LookupRef), + TxFunction(TxFunction), +} + +#[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] +pub enum AttributePlace { + Entid(Entid), } #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] @@ -114,14 +121,14 @@ pub enum OpType { } #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] -pub enum Entity { +pub enum Entity { // Like [:db/add|:db/retract e a v]. AddOrRetract { op: OpType, - e: EntidOrLookupRefOrTempId, - a: Entid, - v: AtomOrLookupRefOrVectorOrMapNotation, + e: EntityPlace, + a: AttributePlace, + v: ValuePlace, }, // Like {:db/id "tempid" a1 v1 a2 v2}. - MapNotation(MapNotation), + MapNotation(MapNotation), } diff --git a/src/conn.rs b/src/conn.rs index c694aa9a..43d5a57e 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -69,6 +69,7 @@ use mentat_db::{ transact_terms, InProgressObserverTransactWatcher, PartitionMap, + TransactableValue, TransactWatcher, TxObservationService, TxObserver, @@ -82,9 +83,7 @@ use mentat_query_pull::{ pull_attributes_for_entity, }; -use mentat_tx; - -use mentat_tx::entities::{ +use edn::entities::{ TempId, OpType, }; @@ -469,7 +468,7 @@ impl<'a, 'c> InProgress<'a, 'c> { Ok(report) } - pub fn transact_entities(&mut self, entities: I) -> Result where I: IntoIterator { + pub fn transact_entities(&mut self, entities: I) -> Result where I: IntoIterator> { // We clone the partition map here, rather than trying to use a Cell or using a mutable // reference, for two reasons: // 1. `transact` allocates new IDs in partitions before and while doing work that might