From 4acc6d065874b9da12db1255fd9091bc5e0ef127 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 23 Jan 2018 08:31:27 -0800 Subject: [PATCH] InProgressRead, KnownEntid. r=nalexander,emily Improve naming of read-only transactions. Implement entid_for_type. Simplify get_attribute. Name ignored var in algebrizer. Comment attribute_for_ident. Make KnownEntid a core concept. Expose lookup_value_for_attribute. Implement HasSchema and a new query encapsulation on Conn. Pre: export Queryable. --- core/src/lib.rs | 66 ++++++--- db/src/internal_types.rs | 6 +- db/src/schema.rs | 7 +- db/src/tx.rs | 9 +- query-algebrizer/src/clauses/convert.rs | 2 +- query-algebrizer/src/clauses/fulltext.rs | 2 +- query-algebrizer/src/clauses/mod.rs | 17 ++- query-algebrizer/src/clauses/pattern.rs | 8 +- src/conn.rs | 171 +++++++++++++++++++---- src/errors.rs | 10 +- src/lib.rs | 1 + src/query.rs | 41 +++--- tests/query.rs | 5 +- 13 files changed, 251 insertions(+), 94 deletions(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index bb62c266..316eab17 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -55,6 +55,23 @@ pub use edn::{ /// use i64 rather than manually truncating u64 to u63 and casting to i64 throughout the codebase. pub type Entid = i64; +/// An entid that's either already in the store, or newly allocated to a tempid. +/// TODO: we'd like to link this in some way to the lifetime of a particular PartitionMap. +#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] +pub struct KnownEntid(pub Entid); + +impl From for Entid { + fn from(k: KnownEntid) -> Entid { + k.0 + } +} + +impl From for TypedValue { + fn from(k: KnownEntid) -> TypedValue { + TypedValue::Ref(k.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)] @@ -673,13 +690,17 @@ pub struct Schema { } pub trait HasSchema { - fn get_ident(&self, x: Entid) -> Option<&NamespacedKeyword>; - fn get_entid(&self, x: &NamespacedKeyword) -> Option; - fn attribute_for_entid(&self, x: Entid) -> Option<&Attribute>; - fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<&Attribute>; + fn entid_for_type(&self, t: ValueType) -> Option; + + fn get_ident(&self, x: T) -> Option<&NamespacedKeyword> where T: Into; + fn get_entid(&self, x: &NamespacedKeyword) -> Option; + fn attribute_for_entid(&self, x: T) -> Option<&Attribute> where T: Into; + + // Returns the attribute and the entid named by the provided ident. + fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<(&Attribute, KnownEntid)>; /// Return true if the provided entid identifies an attribute in this schema. - fn is_attribute(&self, x: Entid) -> bool; + fn is_attribute(&self, x: T) -> bool where T: Into; /// Return true if the provided ident identifies an attribute in this schema. fn identifies_attribute(&self, x: &NamespacedKeyword) -> bool; @@ -693,34 +714,45 @@ impl Schema { attribute.to_edn_value(self.get_ident(*entid).cloned())) .collect()) } + + fn get_raw_entid(&self, x: &NamespacedKeyword) -> Option { + self.ident_map.get(x).map(|x| *x) + } } impl HasSchema for Schema { - fn get_ident(&self, x: Entid) -> Option<&NamespacedKeyword> { - self.entid_map.get(&x) + fn entid_for_type(&self, t: ValueType) -> Option { + // TODO: this can be made more efficient. + self.get_entid(&t.into_keyword()) } - fn get_entid(&self, x: &NamespacedKeyword) -> Option { - self.ident_map.get(x).map(|x| *x) + fn get_ident(&self, x: T) -> Option<&NamespacedKeyword> where T: Into { + self.entid_map.get(&x.into()) } - fn attribute_for_entid(&self, x: Entid) -> Option<&Attribute> { - self.attribute_map.get(&x) + fn get_entid(&self, x: &NamespacedKeyword) -> Option { + self.get_raw_entid(x).map(KnownEntid) } - fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<&Attribute> { - self.get_entid(&ident) - .and_then(|x| self.attribute_for_entid(x)) + fn attribute_for_entid(&self, x: T) -> Option<&Attribute> where T: Into { + self.attribute_map.get(&x.into()) + } + + fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<(&Attribute, KnownEntid)> { + self.get_raw_entid(&ident) + .and_then(|entid| { + self.attribute_for_entid(entid).map(|a| (a, KnownEntid(entid))) + }) } /// Return true if the provided entid identifies an attribute in this schema. - fn is_attribute(&self, x: Entid) -> bool { - self.attribute_map.contains_key(&x) + fn is_attribute(&self, x: T) -> bool where T: Into { + self.attribute_map.contains_key(&x.into()) } /// Return true if the provided ident identifies an attribute in this schema. fn identifies_attribute(&self, x: &NamespacedKeyword) -> bool { - self.get_entid(x).map(|e| self.is_attribute(e)).unwrap_or(false) + self.get_raw_entid(x).map(|e| self.is_attribute(e)).unwrap_or(false) } } diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 2955faf1..67b2f3c6 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -15,6 +15,8 @@ use std::collections::HashMap; use std::rc::Rc; +use mentat_core::KnownEntid; + use mentat_core::util::Either; use errors; @@ -37,10 +39,6 @@ pub enum Term { use self::Either::*; -/// An entid that's either already in the store, or newly allocated to a tempid. -#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] -pub struct KnownEntid(pub Entid); - pub type KnownEntidOr = Either; pub type TypedValueOr = Either; diff --git a/db/src/schema.rs b/db/src/schema.rs index 1729b307..cca9d1ac 100644 --- a/db/src/schema.rs +++ b/db/src/schema.rs @@ -21,6 +21,7 @@ use mentat_core::{ EntidMap, HasSchema, IdentMap, + KnownEntid, Schema, AttributeMap, TypedValue, @@ -173,7 +174,7 @@ impl AttributeBuilder { pub trait SchemaBuilding { fn require_ident(&self, entid: Entid) -> Result<&symbols::NamespacedKeyword>; - fn require_entid(&self, ident: &symbols::NamespacedKeyword) -> Result; + fn require_entid(&self, ident: &symbols::NamespacedKeyword) -> Result; fn require_attribute_for_entid(&self, entid: Entid) -> Result<&Attribute>; fn from_ident_map_and_attribute_map(ident_map: IdentMap, attribute_map: AttributeMap) -> Result; fn from_ident_map_and_triples(ident_map: IdentMap, assertions: U) -> Result @@ -185,7 +186,7 @@ impl SchemaBuilding for Schema { self.get_ident(entid).ok_or(ErrorKind::UnrecognizedEntid(entid).into()) } - fn require_entid(&self, ident: &symbols::NamespacedKeyword) -> Result { + fn require_entid(&self, ident: &symbols::NamespacedKeyword) -> Result { self.get_entid(&ident).ok_or(ErrorKind::UnrecognizedIdent(ident.to_string()).into()) } @@ -248,7 +249,7 @@ impl SchemaTypeChecking for Schema { (ValueType::Keyword, tv @ TypedValue::Keyword(_)) => Ok(tv), // Ref coerces a little: we interpret some things depending on the schema as a Ref. (ValueType::Ref, TypedValue::Long(x)) => Ok(TypedValue::Ref(x)), - (ValueType::Ref, TypedValue::Keyword(ref x)) => self.require_entid(&x).map(|entid| TypedValue::Ref(entid)), + (ValueType::Ref, TypedValue::Keyword(ref x)) => self.require_entid(&x).map(|entid| entid.into()), // Otherwise, we have a type mismatch. // Enumerate all of the types here to allow the compiler to help us. diff --git a/db/src/tx.rs b/db/src/tx.rs index 058a0370..744d0e80 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -64,7 +64,6 @@ use edn::{ use entids; use errors::{ErrorKind, Result}; use internal_types::{ - KnownEntid, KnownEntidOr, LookupRef, LookupRefOrTempId, @@ -82,6 +81,7 @@ use mentat_core::util::Either; use mentat_core::{ DateTime, + KnownEntid, Schema, Utc, attribute, @@ -230,14 +230,13 @@ impl<'conn, 'a> Tx<'conn, 'a> { } fn ensure_ident_exists(&self, e: &NamespacedKeyword) -> Result { - let entid = self.schema.require_entid(e)?; - Ok(KnownEntid(entid)) + self.schema.require_entid(e) } 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)?, + entmod::Entid::Ident(ref a) => self.schema.require_entid(&a)?.into(), }; let lr_attribute: &Attribute = self.schema.require_attribute_for_entid(lr_a)?; @@ -283,7 +282,7 @@ impl<'conn, 'a> Tx<'conn, 'a> { fn entity_a_into_term_a(&mut self, x: entmod::Entid) -> Result { let a = match x { entmod::Entid::Entid(ref a) => *a, - entmod::Entid::Ident(ref a) => self.schema.require_entid(&a)?, + entmod::Entid::Ident(ref a) => self.schema.require_entid(&a)?.into(), }; Ok(a) } diff --git a/query-algebrizer/src/clauses/convert.rs b/query-algebrizer/src/clauses/convert.rs index d0ff6d66..ed528751 100644 --- a/query-algebrizer/src/clauses/convert.rs +++ b/query-algebrizer/src/clauses/convert.rs @@ -170,7 +170,7 @@ impl ConjoiningClauses { }, (true, false) => { // This can only be an ident. Look it up! - match schema.get_entid(&x).map(TypedValue::Ref) { + match schema.get_entid(&x).map(|k| k.into()) { Some(e) => Ok(Val(e)), None => Ok(Impossible(EmptyBecause::UnresolvedIdent(x.clone()))), } diff --git a/query-algebrizer/src/clauses/fulltext.rs b/query-algebrizer/src/clauses/fulltext.rs index e4e41abd..60273796 100644 --- a/query-algebrizer/src/clauses/fulltext.rs +++ b/query-algebrizer/src/clauses/fulltext.rs @@ -102,7 +102,7 @@ impl ConjoiningClauses { // // TODO: improve the expression of this matching, possibly by using attribute_for_* uniformly. let a = match args.next().unwrap() { - FnArg::IdentOrKeyword(i) => schema.get_entid(&i), + FnArg::IdentOrKeyword(i) => schema.get_entid(&i).map(|k| k.into()), // Must be an entid. FnArg::EntidOrInteger(e) => Some(e), FnArg::Variable(v) => { diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index f0520d5a..8764a85d 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -26,6 +26,7 @@ use mentat_core::{ Attribute, Entid, HasSchema, + KnownEntid, Schema, TypedValue, ValueType, @@ -441,7 +442,7 @@ impl ConjoiningClauses { match bound_val { TypedValue::Keyword(ref kw) => { if let Some(entid) = self.entid_for_ident(schema, kw) { - self.constrain_column_to_entity(table, column, entid); + self.constrain_column_to_entity(table, column, entid.into()); } else { // Impossible. // For attributes this shouldn't occur, because we check the binding in @@ -649,7 +650,7 @@ impl ConjoiningClauses { self.empty_because = Some(why); } - fn entid_for_ident<'s, 'a>(&self, schema: &'s Schema, ident: &'a NamespacedKeyword) -> Option { + fn entid_for_ident<'s, 'a>(&self, schema: &'s Schema, ident: &'a NamespacedKeyword) -> Option { schema.get_entid(&ident) } @@ -714,7 +715,7 @@ impl ConjoiningClauses { &PatternNonValuePlace::Ident(ref kw) => schema.attribute_for_ident(kw) .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.cloned())) - .and_then(|attribute| self.table_for_attribute_and_value(attribute, value)), + .and_then(|(attribute, _entid)| self.table_for_attribute_and_value(attribute, value)), &PatternNonValuePlace::Entid(id) => schema.attribute_for_entid(id) .ok_or_else(|| EmptyBecause::InvalidAttributeEntid(id)) @@ -737,7 +738,7 @@ impl ConjoiningClauses { // Don't recurse: avoid needing to clone the keyword. schema.attribute_for_ident(kw) .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.cloned())) - .and_then(|attribute| self.table_for_attribute_and_value(attribute, value)), + .and_then(|(attribute, _entid)| self.table_for_attribute_and_value(attribute, value)), Some(v) => { // This pattern cannot match: the caller has bound a non-entity value to an // attribute place. @@ -772,8 +773,9 @@ impl ConjoiningClauses { fn get_attribute_for_value<'s>(&self, schema: &'s Schema, value: &TypedValue) -> Option<&'s Attribute> { match value { + // We know this one is known if the attribute lookup succeeds… &TypedValue::Ref(id) => schema.attribute_for_entid(id), - &TypedValue::Keyword(ref kw) => schema.attribute_for_ident(kw), + &TypedValue::Keyword(ref kw) => schema.attribute_for_ident(kw).map(|(a, _id)| a), _ => None, } } @@ -781,9 +783,10 @@ impl ConjoiningClauses { fn get_attribute<'s, 'a>(&self, schema: &'s Schema, pattern: &'a Pattern) -> Option<&'s Attribute> { match pattern.attribute { PatternNonValuePlace::Entid(id) => + // We know this one is known if the attribute lookup succeeds… schema.attribute_for_entid(id), PatternNonValuePlace::Ident(ref kw) => - schema.attribute_for_ident(kw), + schema.attribute_for_ident(kw).map(|(a, _id)| a), PatternNonValuePlace::Variable(ref var) => // If the pattern has a variable, we've already determined that the binding -- if // any -- is acceptable and yields a table. Here, simply look to see if it names @@ -796,7 +799,7 @@ impl ConjoiningClauses { } fn get_value_type<'s, 'a>(&self, schema: &'s Schema, pattern: &'a Pattern) -> Option { - self.get_attribute(schema, pattern).map(|x| x.value_type) + self.get_attribute(schema, pattern).map(|a| a.value_type) } } diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index 02a2f275..9ec9eee9 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -100,7 +100,7 @@ impl ConjoiningClauses { self.constrain_column_to_entity(col.clone(), DatomsColumn::Entity, entid), PatternNonValuePlace::Ident(ref ident) => { if let Some(entid) = self.entid_for_ident(schema, ident.as_ref()) { - self.constrain_column_to_entity(col.clone(), DatomsColumn::Entity, entid) + self.constrain_column_to_entity(col.clone(), DatomsColumn::Entity, entid.into()) } else { // A resolution failure means we're done here. self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.cloned())); @@ -125,7 +125,7 @@ impl ConjoiningClauses { }, PatternNonValuePlace::Ident(ref ident) => { if let Some(entid) = self.entid_for_ident(schema, ident) { - self.constrain_attribute(col.clone(), entid); + self.constrain_attribute(col.clone(), entid.into()); if !schema.is_attribute(entid) { self.mark_known_empty(EmptyBecause::InvalidAttributeIdent(ident.cloned())); @@ -191,7 +191,7 @@ impl ConjoiningClauses { // such. if let Some(ValueType::Ref) = value_type { if let Some(entid) = self.entid_for_ident(schema, kw) { - self.constrain_column_to_entity(col.clone(), DatomsColumn::Value, entid) + self.constrain_column_to_entity(col.clone(), DatomsColumn::Value, entid.into()) } else { // A resolution failure means we're done here: this attribute must have an // entity value. @@ -252,7 +252,7 @@ impl ConjoiningClauses { }, PatternNonValuePlace::Ident(ref ident) => { if let Some(entid) = self.entid_for_ident(schema, ident.as_ref()) { - self.constrain_column_to_entity(col.clone(), DatomsColumn::Tx, entid) + self.constrain_column_to_entity(col.clone(), DatomsColumn::Tx, entid.into()) } else { // A resolution failure means we're done here. self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.cloned())); diff --git a/src/conn.rs b/src/conn.rs index 9aa5ff07..53b11716 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -20,9 +20,14 @@ use rusqlite::{ use edn; use mentat_core::{ + Attribute, Entid, + HasSchema, + KnownEntid, + NamespacedKeyword, Schema, TypedValue, + ValueType, }; use mentat_db::db; @@ -39,6 +44,7 @@ use mentat_tx_parser; use errors::*; use query::{ lookup_value_for_attribute, + lookup_values_for_attribute, q_once, q_explain, QueryExplanation, @@ -88,6 +94,15 @@ pub struct Conn { // the schema changes. #315. } +pub trait Queryable { + fn q_once(&self, query: &str, inputs: T) -> Result + where T: Into>; + fn lookup_values_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into; + fn lookup_value_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into; +} + /// Represents an in-progress, not yet committed, set of changes to the store. /// Call `commit` to commit your changes, or `rollback` to discard them. /// A transaction is held open until you do so. @@ -101,6 +116,112 @@ pub struct InProgress<'a, 'c> { last_report: Option, // For now we track only the last, but we could accumulate all. } +/// Represents an in-progress set of reads to the store. Just like `InProgress`, +/// which is read-write, but only allows for reads. +pub struct InProgressRead<'a, 'c>(InProgress<'a, 'c>); + +impl<'a, 'c> Queryable for InProgressRead<'a, 'c> { + fn q_once(&self, query: &str, inputs: T) -> Result + where T: Into> { + self.0.q_once(query, inputs) + } + + fn lookup_values_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into { + self.0.lookup_values_for_attribute(entity, attribute) + } + + fn lookup_value_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into { + self.0.lookup_value_for_attribute(entity, attribute) + } +} + +impl<'a, 'c> Queryable for InProgress<'a, 'c> { + fn q_once(&self, query: &str, inputs: T) -> Result + where T: Into> { + + q_once(&*(self.transaction), + &self.schema, + query, + inputs) + } + + fn lookup_values_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into { + lookup_values_for_attribute(&*(self.transaction), &self.schema, entity, attribute) + } + + fn lookup_value_for_attribute(&self, entity: E, attribute: &edn::NamespacedKeyword) -> Result> + where E: Into { + lookup_value_for_attribute(&*(self.transaction), &self.schema, entity, attribute) + } +} + +impl<'a, 'c> HasSchema for InProgressRead<'a, 'c> { + fn entid_for_type(&self, t: ValueType) -> Option { + self.0.entid_for_type(t) + } + + fn get_ident(&self, x: T) -> Option<&NamespacedKeyword> where T: Into { + self.0.get_ident(x) + } + + fn get_entid(&self, x: &NamespacedKeyword) -> Option { + self.0.get_entid(x) + } + + fn attribute_for_entid(&self, x: T) -> Option<&Attribute> where T: Into { + self.0.attribute_for_entid(x) + } + + fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<(&Attribute, KnownEntid)> { + self.0.attribute_for_ident(ident) + } + + /// Return true if the provided entid identifies an attribute in this schema. + fn is_attribute(&self, x: T) -> bool where T: Into { + self.0.is_attribute(x) + } + + /// Return true if the provided ident identifies an attribute in this schema. + fn identifies_attribute(&self, x: &NamespacedKeyword) -> bool { + self.0.identifies_attribute(x) + } +} + +impl<'a, 'c> HasSchema for InProgress<'a, 'c> { + fn entid_for_type(&self, t: ValueType) -> Option { + self.schema.entid_for_type(t) + } + + fn get_ident(&self, x: T) -> Option<&NamespacedKeyword> where T: Into { + self.schema.get_ident(x) + } + + fn get_entid(&self, x: &NamespacedKeyword) -> Option { + self.schema.get_entid(x) + } + + fn attribute_for_entid(&self, x: T) -> Option<&Attribute> where T: Into { + self.schema.attribute_for_entid(x) + } + + fn attribute_for_ident(&self, ident: &NamespacedKeyword) -> Option<(&Attribute, KnownEntid)> { + self.schema.attribute_for_ident(ident) + } + + /// Return true if the provided entid identifies an attribute in this schema. + fn is_attribute(&self, x: T) -> bool where T: Into { + self.schema.is_attribute(x) + } + + /// Return true if the provided ident identifies an attribute in this schema. + fn identifies_attribute(&self, x: &NamespacedKeyword) -> bool { + self.schema.identifies_attribute(x) + } +} + impl<'a, 'c> InProgress<'a, 'c> { 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 @@ -120,25 +241,6 @@ impl<'a, 'c> InProgress<'a, 'c> { Ok(()) } - /// Query the Mentat store, using the given connection and the current metadata. - pub fn q_once(&self, - query: &str, - inputs: T) -> Result - where T: Into> - { - - q_once(&*(self.transaction), - &self.schema, - query, - inputs) - } - - pub fn lookup_value_for_attribute(&self, - entity: Entid, - attribute: &edn::NamespacedKeyword) -> Result> { - lookup_value_for_attribute(&*(self.transaction), &self.schema, entity, attribute) - } - pub fn transact(&mut self, transaction: &str) -> Result<()> { let assertion_vector = edn::parse::value(transaction)?; let entities = mentat_tx_parser::Tx::parse(&assertion_vector)?; @@ -193,7 +295,7 @@ impl Conn { Ok(Conn::new(db.partition_map, db.schema)) } - /// Yield the current `Schema` instance. + /// Yield a clone of the current `Schema` instance. pub fn current_schema(&self) -> Arc { // We always unwrap the mutex lock: if it's poisoned, this will propogate panics to all // accessing threads. This is perhaps not reasonable; we expect the mutex to be held for @@ -215,11 +317,11 @@ impl Conn { sqlite: &rusqlite::Connection, query: &str, inputs: T) -> Result - where T: Into> - { + where T: Into> { + let metadata = self.metadata.lock().unwrap(); q_once(sqlite, - &*self.current_schema(), + &*metadata.schema, // Doesn't clone, unlike `current_schema`. query, inputs) } @@ -241,12 +343,8 @@ impl Conn { } /// Take a SQLite transaction. - /// IMMEDIATE means 'start the transaction now, but don't exclude readers'. It prevents other - /// connections from taking immediate or exclusive transactions. This is appropriate for our - /// writes and `InProgress`: it means we are ready to write whenever we want to, and nobody else - /// can start a transaction that's not `DEFERRED`, but we don't need exclusivity yet. - pub fn begin_transaction<'m, 'conn>(&'m mut self, sqlite: &'conn mut rusqlite::Connection) -> Result> { - let tx = sqlite.transaction_with_behavior(TransactionBehavior::Immediate)?; + fn begin_transaction_with_behavior<'m, 'conn>(&'m mut self, sqlite: &'conn mut rusqlite::Connection, behavior: TransactionBehavior) -> Result> { + let tx = sqlite.transaction_with_behavior(behavior)?; let (current_generation, current_partition_map, current_schema) = { // The mutex is taken during this block. @@ -268,6 +366,21 @@ impl Conn { }) } + // Helper to avoid passing connections around. + // Make both args mutable so that we can't have parallel access. + pub fn begin_read<'m, 'conn>(&'m mut self, sqlite: &'conn mut rusqlite::Connection) -> Result> { + self.begin_transaction_with_behavior(sqlite, TransactionBehavior::Deferred) + .map(InProgressRead) + } + + /// IMMEDIATE means 'start the transaction now, but don't exclude readers'. It prevents other + /// connections from taking immediate or exclusive transactions. This is appropriate for our + /// writes and `InProgress`: it means we are ready to write whenever we want to, and nobody else + /// can start a transaction that's not `DEFERRED`, but we don't need exclusivity yet. + pub fn begin_transaction<'m, 'conn>(&'m mut self, sqlite: &'conn mut rusqlite::Connection) -> Result> { + self.begin_transaction_with_behavior(sqlite, TransactionBehavior::Immediate) + } + /// Transact entities against the Mentat store, using the given connection and the current /// metadata. pub fn transact(&mut self, diff --git a/src/errors.rs b/src/errors.rs index 72023e8e..a7e8f3b9 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -16,7 +16,6 @@ use std::collections::BTreeSet; use edn; use mentat_db; -use mentat_query; use mentat_query_algebrizer; use mentat_query_parser; use mentat_query_projector; @@ -55,9 +54,14 @@ error_chain! { display("invalid argument name: '{}'", name) } - UnknownAttribute(kw: mentat_query::NamespacedKeyword) { + UnknownAttribute(name: String) { description("unknown attribute") - display("unknown attribute: '{}'", kw) + display("unknown attribute: '{}'", name) + } + + InvalidVocabularyVersion { + description("invalid vocabulary version") + display("invalid vocabulary version") } } } diff --git a/src/lib.rs b/src/lib.rs index 4c07927a..17885bc5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -68,6 +68,7 @@ pub use query::{ pub use conn::{ Conn, Metadata, + Queryable, }; #[cfg(test)] diff --git a/src/query.rs b/src/query.rs index f6f2f1bd..3a05b2a3 100644 --- a/src/query.rs +++ b/src/query.rs @@ -16,6 +16,7 @@ use std::rc::Rc; use mentat_core::{ Entid, HasSchema, + KnownEntid, Schema, TypedValue, }; @@ -159,49 +160,53 @@ fn fetch_values<'sqlite, 'schema> run_algebrized_query(sqlite, algebrized) } -fn lookup_attribute(schema: &Schema, attribute: &NamespacedKeyword) -> Result { +fn lookup_attribute(schema: &Schema, attribute: &NamespacedKeyword) -> Result { schema.get_entid(attribute) - .ok_or_else(|| ErrorKind::UnknownAttribute(attribute.clone()).into()) + .ok_or_else(|| ErrorKind::UnknownAttribute(attribute.name.clone()).into()) } /// Return a single value for the provided entity and attribute. /// If the attribute is multi-valued, an arbitrary value is returned. /// If no value is present for that entity, `None` is returned. /// If `attribute` isn't an attribute, `None` is returned. -pub fn lookup_value<'sqlite, 'schema> +pub fn lookup_value<'sqlite, 'schema, E, A> (sqlite: &'sqlite rusqlite::Connection, schema: &'schema Schema, - entity: Entid, - attribute: Entid) -> Result> { - fetch_values(sqlite, schema, entity, attribute, true).into_scalar_result() + entity: E, + attribute: A) -> Result> + where E: Into, A: Into { + fetch_values(sqlite, schema, entity.into(), attribute.into(), true).into_scalar_result() } -pub fn lookup_values<'sqlite, 'schema> +pub fn lookup_values<'sqlite, 'schema, E, A> (sqlite: &'sqlite rusqlite::Connection, schema: &'schema Schema, - entity: Entid, - attribute: Entid) -> Result> { - fetch_values(sqlite, schema, entity, attribute, false).into_coll_result() + entity: E, + attribute: A) -> Result> + where E: Into, A: Into { + fetch_values(sqlite, schema, entity.into(), attribute.into(), false).into_coll_result() } /// Return a single value for the provided entity and attribute. /// If the attribute is multi-valued, an arbitrary value is returned. /// If no value is present for that entity, `None` is returned. /// If `attribute` doesn't name an attribute, an error is returned. -pub fn lookup_value_for_attribute<'sqlite, 'schema, 'attribute> +pub fn lookup_value_for_attribute<'sqlite, 'schema, 'attribute, E> (sqlite: &'sqlite rusqlite::Connection, schema: &'schema Schema, - entity: Entid, - attribute: &'attribute NamespacedKeyword) -> Result> { - lookup_value(sqlite, schema, entity, lookup_attribute(schema, attribute)?) + entity: E, + attribute: &'attribute NamespacedKeyword) -> Result> + where E: Into { + lookup_value(sqlite, schema, entity.into(), lookup_attribute(schema, attribute)?) } -pub fn lookup_values_for_attribute<'sqlite, 'schema, 'attribute> +pub fn lookup_values_for_attribute<'sqlite, 'schema, 'attribute, E> (sqlite: &'sqlite rusqlite::Connection, schema: &'schema Schema, - entity: Entid, - attribute: &'attribute NamespacedKeyword) -> Result> { - lookup_values(sqlite, schema, entity, lookup_attribute(schema, attribute)?) + entity: E, + attribute: &'attribute NamespacedKeyword) -> Result> + where E: Into { + lookup_values(sqlite, schema, entity.into(), lookup_attribute(schema, attribute)?) } fn run_statement<'sqlite, 'stmt, 'bound> diff --git a/tests/query.rs b/tests/query.rs index 5245467c..4af4bfc8 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -23,6 +23,7 @@ use chrono::FixedOffset; use mentat_core::{ DateTime, HasSchema, + KnownEntid, TypedValue, ValueType, Utc, @@ -115,7 +116,7 @@ fn test_scalar() { if let QueryResults::Scalar(Some(TypedValue::Keyword(ref rc))) = results { // Should be '24'. assert_eq!(&NamespacedKeyword::new("db.type", "keyword"), rc.as_ref()); - assert_eq!(24, + assert_eq!(KnownEntid(24), db.schema.get_entid(rc).unwrap()); } else { panic!("Expected scalar."); @@ -146,7 +147,7 @@ fn test_tuple() { let cardinality_one = NamespacedKeyword::new("db.cardinality", "one"); assert_eq!(tuple.len(), 2); assert_eq!(tuple[0], TypedValue::Boolean(true)); - assert_eq!(tuple[1], TypedValue::Ref(db.schema.get_entid(&cardinality_one).unwrap())); + assert_eq!(tuple[1], db.schema.get_entid(&cardinality_one).expect("c1").into()); } else { panic!("Expected tuple."); }