diff --git a/db/src/db.rs b/db/src/db.rs index 9aa773b9..cced8dfd 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -2087,4 +2087,127 @@ mod tests { // dangling, if we give a :db/id explicitly. assert_transact!(conn, "[{:test/dangling {:db/id \"t\" :test/many 11}}]"); } + + #[test] + fn test_explode_reversed_notation() { + let mut conn = TestConn::default(); + + // Start by installing a few attributes. + assert_transact!(conn, "[[:db/add 111 :db/ident :test/many] + [:db/add 111 :db/valueType :db.type/long] + [:db/add 111 :db/cardinality :db.cardinality/many] + [:db/add 222 :db/ident :test/component] + [:db/add 222 :db/isComponent true] + [:db/add 222 :db/valueType :db.type/ref] + [:db/add 333 :db/ident :test/unique] + [:db/add 333 :db/unique :db.unique/identity] + [:db/add 333 :db/index true] + [:db/add 333 :db/valueType :db.type/long] + [:db/add 444 :db/ident :test/dangling] + [:db/add 444 :db/valueType :db.type/ref]]"); + + // Check that we can explode direct reversed notation, entids. + let report = assert_transact!(conn, "[[:db/add 100 :test/_dangling 200]]"); + assert_matches!(conn.last_transaction(), + "[[200 :test/dangling 100 ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + assert_matches!(tempids(&report), + "{}"); + + // Check that we can explode direct reversed notation, idents. + let report = assert_transact!(conn, "[[:db/add :test/many :test/_dangling :test/unique]]"); + assert_matches!(conn.last_transaction(), + "[[333 :test/dangling :test/many ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + assert_matches!(tempids(&report), + "{}"); + + // Check that we can explode direct reversed notation, tempids. + let report = assert_transact!(conn, "[[:db/add \"s\" :test/_dangling \"t\"]]"); + assert_matches!(conn.last_transaction(), + "[[65537 :test/dangling 65536 ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + // This is implementation specific, but it should be deterministic. + assert_matches!(tempids(&report), + "{\"s\" 65536 + \"t\" 65537}"); + + // Check that we can explode reversed notation in map notation without :db/id. + let report = assert_transact!(conn, "[{:test/_dangling 501} + {:test/_dangling :test/many} + {:test/_dangling \"t\"}]"); + assert_matches!(conn.last_transaction(), + "[[111 :test/dangling ?e1 ?tx true] + [501 :test/dangling ?e2 ?tx true] + [65538 :test/dangling ?e3 ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + assert_matches!(tempids(&report), + "{\"t\" 65538}"); + + // Check that we can explode reversed notation in map notation with :db/id, entid. + let report = assert_transact!(conn, "[{:db/id 600 :test/_dangling 601}]"); + assert_matches!(conn.last_transaction(), + "[[601 :test/dangling 600 ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + assert_matches!(tempids(&report), + "{}"); + + // Check that we can explode reversed notation in map notation with :db/id, ident. + let report = assert_transact!(conn, "[{:db/id :test/component :test/_dangling :test/component}]"); + assert_matches!(conn.last_transaction(), + "[[222 :test/dangling :test/component ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + assert_matches!(tempids(&report), + "{}"); + + // Check that we can explode reversed notation in map notation with :db/id, tempid. + let report = assert_transact!(conn, "[{:db/id \"s\" :test/_dangling \"t\"}]"); + assert_matches!(conn.last_transaction(), + "[[65543 :test/dangling 65542 ?tx true] + [?tx :db/txInstant ?ms ?tx true]]"); + // This is implementation specific, but it should be deterministic. + assert_matches!(tempids(&report), + "{\"s\" 65542 + \"t\" 65543}"); + + // Verify that we can't explode direct reverse notation with nested value maps. + assert_transact!(conn, + "[[:db/add \"t\" :test/_dangling {:test/many 11}]]", + Err("not yet implemented: Cannot explode map notation value in :attr/_reversed notation for attribute 444")); + + // Verify that we can't explode reverse notation in map notation with nested value maps. + assert_transact!(conn, + "[{:test/_dangling {:test/many 11}}]", + Err("not yet implemented: Cannot explode map notation value in :attr/_reversed notation for attribute 444")); + + // Verify that we can't explode direct reverse notation with nested value vectors. + assert_transact!(conn, + "[[:db/add \"t\" :test/_dangling [:test/many]]]", + Err("not yet implemented: Cannot explode vector value in :attr/_reversed notation for attribute 444")); + + // Verify that we can't explode reverse notation in map notation with nested value vectors. + assert_transact!(conn, + "[{:test/_dangling [:test/many]}]", + Err("not yet implemented: Cannot explode vector value in :attr/_reversed notation for attribute 444")); + + // Verify that we can't use reverse notation with non-:db.type/ref attributes. + assert_transact!(conn, + "[{:test/_unique 500}]", + Err("not yet implemented: Cannot use :attr/_reversed notation for attribute 333 that is not :db/valueType :db.type/ref")); + + // Verify that we can't use reverse notation with unrecognized attributes. + assert_transact!(conn, + "[{:test/_unknown 500}]", + Err("no entid found for ident: :test/unknown")); // TODO: make this error reference the original :test/_unknown. + + // Verify that we can't use reverse notation with bad value types: here, an unknown keyword + // that can't be coerced to a ref. + assert_transact!(conn, + "[{:test/_dangling :test/unknown}]", + Err("no entid found for ident: :test/unknown")); + // And here, a float. + assert_transact!(conn, + "[{:test/_dangling 1.23}]", + Err("EDN value \'1.23\' is not the expected Mentat value type Ref")); + } } diff --git a/db/src/tx.rs b/db/src/tx.rs index 6cdf30a8..e755d292 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -269,6 +269,53 @@ impl<'conn, 'a> Tx<'conn, 'a> { fn entity_e_into_term_v(&mut self, x: entmod::EntidOrLookupRefOrTempId) -> Result> { self.entity_e_into_term_e(x).map(|r| r.map_left(TypedValue::Ref)) } + + fn entity_v_into_term_e(&mut self, x: entmod::AtomOrLookupRefOrVectorOrMapNotation, backward_a: &entmod::Entid) -> Result> { + let reversed_a = match backward_a { + &entmod::Entid::Entid(a) => { + // Programmer error. + unreachable!("Expected ident backward attribute but got entid {}", a) + }, + &entmod::Entid::Ident(ref a) => { + entmod::Entid::Ident(a.to_reversed()) + }, + }; + let (reversed_a, reversed_attribute) = self.entity_a_into_term_a(reversed_a)?; + + if reversed_attribute.value_type != ValueType::Ref { + bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} that is not :db/valueType :db.type/ref", reversed_a))) + } + + match x { + entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(ref 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. + if let Some(text) = v.inner.as_text() { + Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(TempId::External(text.clone()))))) + } else { + // 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. + if let TypedValue::Ref(entid) = self.schema.to_typed_value(&v.clone().without_spans(), &reversed_attribute)? { + Ok(Either::Left(entid)) + } else { + // reversed_attribute is :db.type/ref, so this shouldn't happen. + unreachable!() + } + } + }, + + entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(ref lookup_ref) => + Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))), + + entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) => + bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", reversed_a))), + + entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) => + bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", reversed_a))), + } + } } let mut in_process = InProcess::with_schema(&self.schema); @@ -301,7 +348,13 @@ impl<'conn, 'a> Tx<'conn, 'a> { }, Entity::AddOrRetract { op, e, a, v } => { - let (a, attribute) = in_process.entity_a_into_term_a(a)?; + if a.is_backward() { + let reversed_e = in_process.entity_v_into_term_e(v, &a)?; + let (reversed_a, _reversed_attribute) = in_process.entity_a_into_term_a(a.to_reversed().unwrap())?; + let reversed_v = in_process.entity_e_into_term_v(e)?; + terms.push(Term::AddOrRetract(OpType::Add, reversed_e, reversed_a, reversed_v)); + } else { + let (a, attribute) = in_process.entity_a_into_term_a(a)?; let v = match v { entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { @@ -373,18 +426,32 @@ impl<'conn, 'a> Tx<'conn, 'a> { } for (inner_a, inner_v) in map_notation { - let (inner_a, inner_attribute) = in_process.entity_a_into_term_a(inner_a)?; - - if inner_attribute.unique == Some(attribute::Unique::Identity) { + if inner_a.is_backward() { + // We definitely have a reference. The reference might be + // dangling (a bare entid, for example), but we don't yet + // support nested maps and reverse notation simultaneously + // (i.e., we don't accept {:reverse/_attribute {:nested map}}) + // so we don't need to check that the nested map reference isn't + // danglign. dangling = false; - } - deque.push_front(Entity::AddOrRetract { - op: OpType::Add, - e: db_id.clone(), - a: entmod::Entid::Entid(inner_a), - v: inner_v, - }); + let reversed_e = in_process.entity_v_into_term_e(inner_v, &inner_a)?; + let (reversed_a, _reversed_attribute) = in_process.entity_a_into_term_a(inner_a.to_reversed().unwrap())?; + let reversed_v = in_process.entity_e_into_term_v(db_id.clone())?; + terms.push(Term::AddOrRetract(OpType::Add, reversed_e, reversed_a, reversed_v)); + } else { + let (inner_a, inner_attribute) = in_process.entity_a_into_term_a(inner_a)?; + if inner_attribute.unique == Some(attribute::Unique::Identity) { + dangling = false; + } + + deque.push_front(Entity::AddOrRetract { + op: OpType::Add, + e: db_id.clone(), + a: entmod::Entid::Entid(inner_a), + v: inner_v, + }); + } } if dangling { @@ -397,6 +464,7 @@ impl<'conn, 'a> Tx<'conn, 'a> { let e = in_process.entity_e_into_term_e(e)?; terms.push(Term::AddOrRetract(op, e, a, v)); + } }, } }; diff --git a/tx/src/entities.rs b/tx/src/entities.rs index a943e49c..96ee2716 100644 --- a/tx/src/entities.rs +++ b/tx/src/entities.rs @@ -56,6 +56,22 @@ pub enum Entid { Ident(NamespacedKeyword), } +impl Entid { + pub fn is_backward(&self) -> bool { + match self { + &Entid::Entid(_) => false, + &Entid::Ident(ref a) => a.is_backward(), + } + } + + pub fn to_reversed(&self) -> Option { + match self { + &Entid::Entid(_) => None, + &Entid::Ident(ref a) => Some(Entid::Ident(a.to_reversed())), + } + } +} + #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub struct LookupRef { pub a: Entid,