diff --git a/db/src/db.rs b/db/src/db.rs index 6318b2f6..a0ed1ffb 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -499,7 +499,7 @@ fn read_ident_map(conn: &rusqlite::Connection) -> Result { fn read_attribute_map(conn: &rusqlite::Connection) -> Result { let entid_triples = read_materialized_view(conn, "schema")?; let mut attribute_map = AttributeMap::default(); - metadata::update_attribute_map_from_entid_triples(&mut attribute_map, entid_triples)?; + metadata::update_attribute_map_from_entid_triples(&mut attribute_map, entid_triples, ::std::iter::empty())?; Ok(attribute_map) } @@ -1087,8 +1087,8 @@ SELECT EXISTS // error message in this case. if unique_value_stmt.execute(&[to_bool_ref(attribute.unique.is_some()), &entid as &ToSql]).is_err() { match attribute.unique { - Some(attribute::Unique::Value) => bail!(ErrorKind::NotYetImplemented(format!("Cannot alter schema attribute {} to be :db.unique/value", entid))), - Some(attribute::Unique::Identity) => bail!(ErrorKind::NotYetImplemented(format!("Cannot alter schema attribute {} to be :db.unique/identity", entid))), + Some(attribute::Unique::Value) => bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/value", entid))), + Some(attribute::Unique::Identity) => bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/identity", entid))), None => unreachable!(), // This shouldn't happen, even after we support removing :db/unique. } } @@ -1102,7 +1102,7 @@ SELECT EXISTS if !attribute.multival { let mut rows = cardinality_stmt.query(&[&entid as &ToSql])?; if rows.next().is_some() { - bail!(ErrorKind::NotYetImplemented(format!("Cannot alter schema attribute {} to be :db.cardinality/one", entid))); + bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.cardinality/one", entid))); } } }, @@ -1154,6 +1154,7 @@ mod tests { use edn; use mentat_core::{ HasSchema, + NamespacedKeyword, Schema, attribute, }; @@ -1637,7 +1638,7 @@ mod tests { // Cannot retract a characteristic of an installed attribute. assert_transact!(conn, "[[:db/retract 100 :db/cardinality :db.cardinality/many]]", - Err("not yet implemented: Retracting metadata attribute assertions not yet implemented: retracted [e a] pairs [[100 8]]")); + Err("bad schema assertion: Retracting attribute 8 for entity 100 not permitted.")); // Trying to install an attribute without a :db/ident is allowed. assert_transact!(conn, "[[:db/add 101 :db/valueType :db.type/long] @@ -1772,7 +1773,7 @@ mod tests { // We can't always go from :db.cardinality/many to :db.cardinality/one. assert_transact!(conn, "[[:db/add 100 :db/cardinality :db.cardinality/one]]", // TODO: give more helpful error details. - Err("not yet implemented: Cannot alter schema attribute 100 to be :db.cardinality/one")); + Err("schema alteration failed: Cannot alter schema attribute 100 to be :db.cardinality/one")); } #[test] @@ -1790,12 +1791,12 @@ mod tests { // We can't always migrate to be :db.unique/value. assert_transact!(conn, "[[:db/add :test/ident :db/unique :db.unique/value]]", // TODO: give more helpful error details. - Err("not yet implemented: Cannot alter schema attribute 100 to be :db.unique/value")); + Err("schema alteration failed: Cannot alter schema attribute 100 to be :db.unique/value")); // Not even indirectly! assert_transact!(conn, "[[:db/add :test/ident :db/unique :db.unique/identity]]", // TODO: give more helpful error details. - Err("not yet implemented: Cannot alter schema attribute 100 to be :db.unique/identity")); + Err("schema alteration failed: Cannot alter schema attribute 100 to be :db.unique/identity")); // But we can if we make sure there's no repeated [a v] pair. assert_transact!(conn, "[[:db/add 201 :test/ident 2]]"); @@ -1803,6 +1804,19 @@ mod tests { assert_transact!(conn, "[[:db/add :test/ident :db/index true] [:db/add :test/ident :db/unique :db.unique/value] [:db/add :db.part/db :db.alter/attribute 100]]"); + + // We can also retract the uniqueness constraint altogether. + assert_transact!(conn, "[[:db/retract :test/ident :db/unique :db.unique/value]]"); + + // Once we've done so, the schema shows it's not unique… + { + let attr = conn.schema.attribute_for_ident(&NamespacedKeyword::new("test", "ident")).unwrap().0; + assert_eq!(None, attr.unique); + } + + // … and we can add more assertions with duplicate values. + assert_transact!(conn, "[[:db/add 121 :test/ident 1] + [:db/add 221 :test/ident 2]]"); } /// Verify that we can't alter :db/fulltext schema characteristics at all. @@ -1823,7 +1837,7 @@ mod tests { assert_transact!(conn, "[[:db/retract 111 :db/fulltext true]]", - Err("not yet implemented: Retracting metadata attribute assertions not yet implemented: retracted [e a] pairs [[111 12]]")); + Err("bad schema assertion: Retracting attribute 12 for entity 111 not permitted.")); assert_transact!(conn, "[[:db/add 222 :db/fulltext true]]", diff --git a/db/src/errors.rs b/db/src/errors.rs index 4242227e..6dbf97f7 100644 --- a/db/src/errors.rs +++ b/db/src/errors.rs @@ -97,5 +97,10 @@ error_chain! { description("cannot reverse-cache non-unique attribute") display("cannot reverse-cache non-unique attribute: {}", attr) } + + SchemaAlterationFailed(t: String) { + description("schema alteration failed") + display("schema alteration failed: {}", t) + } } } diff --git a/db/src/metadata.rs b/db/src/metadata.rs index 9e970979..953a7bd6 100644 --- a/db/src/metadata.rs +++ b/db/src/metadata.rs @@ -27,8 +27,6 @@ use std::collections::{BTreeMap, BTreeSet}; use std::collections::btree_map::Entry; -use itertools::Itertools; // For join(). - use add_retract_alter_set::{ AddRetractAlterSet, }; @@ -104,14 +102,66 @@ impl MetadataReport { /// contain install and alter markers. /// /// Returns a report summarizing the mutations that were applied. -pub fn update_attribute_map_from_entid_triples(attribute_map: &mut AttributeMap, assertions: U) -> Result - where U: IntoIterator { +pub fn update_attribute_map_from_entid_triples(attribute_map: &mut AttributeMap, assertions: A, retractions: R) -> Result + where A: IntoIterator, + R: IntoIterator { + + fn attribute_builder_to_modify(attribute_id: Entid, existing: &AttributeMap) -> AttributeBuilder { + existing.get(&attribute_id) + .map(AttributeBuilder::to_modify_attribute) + .unwrap_or_else(AttributeBuilder::default) + } // Group mutations by impacted entid. let mut builders: BTreeMap = BTreeMap::new(); + // For retractions, we start with an attribute builder that's pre-populated with the existing + // attribute values. That allows us to check existing values and unset them. + for (entid, attr, ref value) in retractions.into_iter() { + let builder = builders.entry(entid).or_insert_with(|| attribute_builder_to_modify(entid, attribute_map)); + match attr { + // You can only retract :db/unique, :db/doc, :db/isComponent; all others + // must be altered instead of retracted, or are not allowed to change. + entids::DB_DOC => { + // Nothing to do here; we don't keep docstrings inside `Attribute`s. + }, + entids::DB_IS_COMPONENT => { + match value { + &TypedValue::Boolean(v) if builder.component == Some(v) => { + builder.component(false); + }, + v => { + bail!(ErrorKind::BadSchemaAssertion(format!("Attempted to retract :db/isComponent with the wrong value {:?}.", v))); + }, + } + }, + entids::DB_UNIQUE => { + match *value { + TypedValue::Ref(u) => { + match u { + entids::DB_UNIQUE_VALUE if builder.unique == Some(Some(attribute::Unique::Value)) => { + builder.non_unique(); + }, + entids::DB_UNIQUE_IDENTITY if builder.unique == Some(Some(attribute::Unique::Identity)) => { + builder.non_unique(); + }, + v => { + bail!(ErrorKind::BadSchemaAssertion(format!("Attempted to retract :db/unique with the wrong value {}.", v))); + }, + } + }, + _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [:db/retract _ :db/unique :db.unique/_] but got [:db/retract {} :db/unique {:?}]", entid, value))) + } + }, + _ => { + bail!(ErrorKind::BadSchemaAssertion(format!("Retracting attribute {} for entity {} not permitted.", attr, entid))); + }, + } + } + for (entid, attr, ref value) in assertions.into_iter() { - let builder = builders.entry(entid).or_insert(AttributeBuilder::default()); + // For assertions, we can start with an empty attribute builder. + let builder = builders.entry(entid).or_insert_with(Default::default); // TODO: improve error messages throughout. match attr { @@ -146,11 +196,6 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut AttributeM entids::DB_UNIQUE => { match *value { - // TODO: accept nil in some form. - // TypedValue::Nil => { - // builder.unique_value(false); - // builder.unique_identity(false); - // }, TypedValue::Ref(entids::DB_UNIQUE_VALUE) => { builder.unique(attribute::Unique::Value); }, TypedValue::Ref(entids::DB_UNIQUE_IDENTITY) => { builder.unique(attribute::Unique::Identity); }, _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/unique :db.unique/value|:db.unique/identity] but got [... :db/unique {:?}]", value))) @@ -257,17 +302,14 @@ pub fn update_schema_from_entid_quadruples(schema: &mut Schema, assertions: U attribute_set.witness((e, a), typed_value, added); } - // Datomic does not allow to retract attributes or idents. For now, Mentat follows suit. - if !attribute_set.retracted.is_empty() { - bail!(ErrorKind::NotYetImplemented(format!("Retracting metadata attribute assertions not yet implemented: retracted [e a] pairs [{}]", - attribute_set.retracted.keys().map(|&(e, a)| format!("[{} {}]", e, a)).join(", ")))); - } - // Collect triples. + let retracted_triples = attribute_set.retracted.into_iter().map(|((e, a), typed_value)| (e, a, typed_value)); let asserted_triples = attribute_set.asserted.into_iter().map(|((e, a), typed_value)| (e, a, typed_value)); let altered_triples = attribute_set.altered.into_iter().map(|((e, a), (_old_value, new_value))| (e, a, new_value)); - let report = update_attribute_map_from_entid_triples(&mut schema.attribute_map, asserted_triples.chain(altered_triples))?; + let report = update_attribute_map_from_entid_triples(&mut schema.attribute_map, + asserted_triples.chain(altered_triples), + retracted_triples)?; let mut idents_altered: BTreeMap = BTreeMap::new(); diff --git a/db/src/schema.rs b/db/src/schema.rs index 6c0d1d27..bfa6589b 100644 --- a/db/src/schema.rs +++ b/db/src/schema.rs @@ -73,13 +73,13 @@ fn validate_attribute_map(entid_map: &EntidMap, attribute_map: &AttributeMap) -> #[derive(Clone,Debug,Default,Eq,Hash,Ord,PartialOrd,PartialEq)] pub struct AttributeBuilder { helpful: bool, - value_type: Option, - multival: Option, - unique: Option>, - index: Option, - fulltext: Option, - component: Option, - no_history: Option, + pub value_type: Option, + pub multival: Option, + pub unique: Option>, + pub index: Option, + pub fulltext: Option, + pub component: Option, + pub no_history: Option, } impl AttributeBuilder { @@ -92,6 +92,16 @@ impl AttributeBuilder { } } + /// Make a new AttributeBuilder from an existing Attribute. This is important to allow + /// retraction. Only attributes that we allow to change are duplicated here. + pub fn to_modify_attribute(attribute: &Attribute) -> Self { + let mut ab = AttributeBuilder::default(); + ab.multival = Some(attribute.multival); + ab.unique = Some(attribute.unique); + ab.component = Some(attribute.component); + ab + } + pub fn value_type<'a>(&'a mut self, value_type: ValueType) -> &'a mut Self { self.value_type = Some(value_type); self @@ -102,6 +112,11 @@ impl AttributeBuilder { self } + pub fn non_unique<'a>(&'a mut self) -> &'a mut Self { + self.unique = Some(None); + self + } + pub fn unique<'a>(&'a mut self, unique: attribute::Unique) -> &'a mut Self { if self.helpful && unique == attribute::Unique::Identity { self.index = Some(true); @@ -185,12 +200,19 @@ impl AttributeBuilder { mutations.push(AttributeAlteration::Cardinality); } } + if let Some(ref unique) = self.unique { if *unique != attribute.unique { attribute.unique = unique.clone(); mutations.push(AttributeAlteration::Unique); } + } else { + if attribute.unique != None { + attribute.unique = None; + mutations.push(AttributeAlteration::Unique); + } } + if let Some(index) = self.index { if index != attribute.index { attribute.index = index; @@ -255,7 +277,10 @@ impl SchemaBuilding for Schema { }).collect(); let mut schema = Schema::from_ident_map_and_attribute_map(ident_map, AttributeMap::default())?; - let metadata_report = metadata::update_attribute_map_from_entid_triples(&mut schema.attribute_map, entid_assertions?)?; + let metadata_report = metadata::update_attribute_map_from_entid_triples(&mut schema.attribute_map, + entid_assertions?, + // No retractions. + ::std::iter::empty())?; // Rebuild the component attributes list if necessary. if metadata_report.attributes_did_change() {