diff --git a/core/src/lib.rs b/core/src/lib.rs index 0cbc365f..d715d64a 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -17,6 +17,7 @@ extern crate edn; pub mod values; use std::collections::BTreeMap; +use std::rc::Rc; use self::ordered_float::OrderedFloat; use self::edn::NamespacedKeyword; @@ -66,8 +67,8 @@ pub enum TypedValue { Long(i64), Double(OrderedFloat), // TODO: &str throughout? - String(String), - Keyword(NamespacedKeyword), + String(Rc), + Keyword(Rc), } impl TypedValue { @@ -95,17 +96,17 @@ impl TypedValue { } /// Construct a new `TypedValue::Keyword` instance by cloning the provided - /// values. This is expensive, so this might + /// values and wrapping them in a new `Rc`. This is expensive, so this might /// be best limited to tests. pub fn typed_ns_keyword(ns: &str, name: &str) -> TypedValue { - TypedValue::Keyword(NamespacedKeyword::new(ns, name)) + TypedValue::Keyword(Rc::new(NamespacedKeyword::new(ns, name))) } /// Construct a new `TypedValue::String` instance by cloning the provided - /// value. This is expensive, so this might + /// value and wrapping it in a new `Rc`. This is expensive, so this might /// be best limited to tests. pub fn typed_string(s: &str) -> TypedValue { - TypedValue::String(s.to_string()) + TypedValue::String(Rc::new(s.to_string())) } } diff --git a/db/src/db.rs b/db/src/db.rs index 5d51717c..d39502f6 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -16,6 +16,7 @@ use std::fmt::Display; use std::iter::{once, repeat}; use std::ops::Range; use std::path::Path; +use std::rc::Rc; use itertools; use itertools::Itertools; @@ -350,9 +351,9 @@ impl TypedSQLValue for TypedValue { // share a tag. (5, rusqlite::types::Value::Integer(x)) => Ok(TypedValue::Long(x)), (5, rusqlite::types::Value::Real(x)) => Ok(TypedValue::Double(x.into())), - (10, rusqlite::types::Value::Text(x)) => Ok(TypedValue::String(x)), + (10, rusqlite::types::Value::Text(x)) => Ok(TypedValue::String(Rc::new(x))), (13, rusqlite::types::Value::Text(x)) => { - to_namespaced_keyword(&x).map(|k| TypedValue::Keyword(k)) + to_namespaced_keyword(&x).map(|k| TypedValue::Keyword(Rc::new(k))) }, (_, value) => bail!(ErrorKind::BadSQLValuePair(value, value_type_tag)), } @@ -370,8 +371,8 @@ impl TypedSQLValue for TypedValue { &Value::Boolean(x) => Some(TypedValue::Boolean(x)), &Value::Integer(x) => Some(TypedValue::Long(x)), &Value::Float(ref x) => Some(TypedValue::Double(x.clone())), - &Value::Text(ref x) => Some(TypedValue::String(x.clone())), - &Value::NamespacedKeyword(ref x) => Some(TypedValue::Keyword(x.clone())), + &Value::Text(ref x) => Some(TypedValue::String(Rc::new(x.clone()))), + &Value::NamespacedKeyword(ref x) => Some(TypedValue::Keyword(Rc::new(x.clone()))), _ => None } } @@ -396,8 +397,8 @@ impl TypedSQLValue for TypedValue { &TypedValue::Boolean(x) => (Value::Boolean(x), ValueType::Boolean), &TypedValue::Long(x) => (Value::Integer(x), ValueType::Long), &TypedValue::Double(x) => (Value::Float(x), ValueType::Double), - &TypedValue::String(ref x) => (Value::Text(x.clone()), ValueType::String), - &TypedValue::Keyword(ref x) => (Value::NamespacedKeyword(x.clone()), ValueType::Keyword), + &TypedValue::String(ref x) => (Value::Text(x.as_ref().clone()), ValueType::String), + &TypedValue::Keyword(ref x) => (Value::NamespacedKeyword(x.as_ref().clone()), ValueType::Keyword), } } } @@ -434,7 +435,7 @@ fn read_ident_map(conn: &rusqlite::Connection) -> Result { bail!(ErrorKind::NotYetImplemented(format!("bad idents materialized view: expected :db/ident but got {}", a))); } if let TypedValue::Keyword(keyword) = typed_value { - Ok((keyword, e)) + Ok((keyword.as_ref().clone(), e)) } else { bail!(ErrorKind::NotYetImplemented(format!("bad idents materialized view: expected [entid :db/ident keyword] but got [entid :db/ident {:?}]", typed_value))); } diff --git a/db/src/debug.rs b/db/src/debug.rs index 2db3d54e..4dd82db2 100644 --- a/db/src/debug.rs +++ b/db/src/debug.rs @@ -14,6 +14,7 @@ use std::borrow::Borrow; use std::io::{Write}; +use std::rc::Rc; use itertools::Itertools; use rusqlite; @@ -110,7 +111,7 @@ trait ToIdent { impl ToIdent for TypedValue { fn map_ident(self, schema: &Schema) -> Self { if let TypedValue::Ref(e) = self { - schema.get_ident(e).cloned().map(TypedValue::Keyword).unwrap_or(TypedValue::Ref(e)) + schema.get_ident(e).cloned().map(|i| TypedValue::Keyword(Rc::new(i))).unwrap_or(TypedValue::Ref(e)) } else { self } diff --git a/db/src/metadata.rs b/db/src/metadata.rs index 1cb08f95..918bdb14 100644 --- a/db/src/metadata.rs +++ b/db/src/metadata.rs @@ -222,7 +222,7 @@ pub fn update_schema_from_entid_quadruples(schema: &mut Schema, assertions: U // Here we handle :db/ident assertions. if a == entids::DB_IDENT { if let TypedValue::Keyword(ref keyword) = typed_value { - ident_set.witness(e, keyword.clone(), added); + ident_set.witness(e, keyword.as_ref().clone(), added); continue } else { // Something is terribly wrong: the schema ensures we have a keyword. diff --git a/db/tests/value_tests.rs b/db/tests/value_tests.rs index 45634c43..8385260f 100644 --- a/db/tests/value_tests.rs +++ b/db/tests/value_tests.rs @@ -14,10 +14,12 @@ extern crate mentat_db; extern crate ordered_float; extern crate rusqlite; +use ordered_float::OrderedFloat; + +use edn::symbols; + use mentat_core::{TypedValue, ValueType}; use mentat_db::db::TypedSQLValue; -use ordered_float::OrderedFloat; -use edn::symbols; // It's not possible to test to_sql_value_pair since rusqlite::ToSqlOutput doesn't implement // PartialEq. @@ -34,8 +36,8 @@ fn test_from_sql_value_pair() { assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Real(0.0), 5).unwrap(), TypedValue::Double(OrderedFloat(0.0))); assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Real(0.5), 5).unwrap(), TypedValue::Double(OrderedFloat(0.5))); - assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Text(":db/keyword".into()), 10).unwrap(), TypedValue::String(":db/keyword".into())); - assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Text(":db/keyword".into()), 13).unwrap(), TypedValue::Keyword(symbols::NamespacedKeyword::new("db", "keyword"))); + assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Text(":db/keyword".into()), 10).unwrap(), TypedValue::typed_string(":db/keyword")); + assert_eq!(TypedValue::from_sql_value_pair(rusqlite::types::Value::Text(":db/keyword".into()), 13).unwrap(), TypedValue::typed_ns_keyword("db", "keyword")); } #[test] @@ -51,6 +53,6 @@ fn test_to_edn_value_pair() { assert_eq!(TypedValue::Double(OrderedFloat(0.0)).to_edn_value_pair(), (edn::Value::Float(OrderedFloat(0.0)), ValueType::Double)); assert_eq!(TypedValue::Double(OrderedFloat(0.5)).to_edn_value_pair(), (edn::Value::Float(OrderedFloat(0.5)), ValueType::Double)); - assert_eq!(TypedValue::String(":db/keyword".into()).to_edn_value_pair(), (edn::Value::Text(":db/keyword".into()), ValueType::String)); - assert_eq!(TypedValue::Keyword(symbols::NamespacedKeyword::new("db", "keyword")).to_edn_value_pair(), (edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("db", "keyword")), ValueType::Keyword)); + assert_eq!(TypedValue::typed_string(":db/keyword").to_edn_value_pair(), (edn::Value::Text(":db/keyword".into()), ValueType::String)); + assert_eq!(TypedValue::typed_ns_keyword("db", "keyword").to_edn_value_pair(), (edn::Value::NamespacedKeyword(symbols::NamespacedKeyword::new("db", "keyword")), ValueType::Keyword)); } diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index f9ee4edc..1872b8f9 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -62,6 +62,17 @@ mod resolve; use validate::validate_or_join; +// We do this a lot for errors. +trait RcCloned { + fn cloned(&self) -> T; +} + +impl RcCloned for ::std::rc::Rc { + fn cloned(&self) -> T { + self.as_ref().clone() + } +} + /// A thing that's capable of aliasing a table name for us. /// This exists so that we can obtain predictable names in tests. pub type TableAliaser = Box TableAlias>; @@ -231,7 +242,7 @@ impl ConjoiningClauses { // For attributes this shouldn't occur, because we check the binding in // `table_for_places`/`alias_table`, and if it didn't resolve to a valid // attribute then we should have already marked the pattern as empty. - self.mark_known_empty(EmptyBecause::UnresolvedIdent(kw.clone())); + self.mark_known_empty(EmptyBecause::UnresolvedIdent(kw.cloned())); } }, TypedValue::Ref(entid) => { @@ -438,7 +449,7 @@ impl ConjoiningClauses { match attribute { &PatternNonValuePlace::Ident(ref kw) => schema.attribute_for_ident(kw) - .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.clone())) + .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.cloned())) .and_then(|attribute| self.table_for_attribute_and_value(attribute, value)), &PatternNonValuePlace::Entid(id) => schema.attribute_for_entid(id) @@ -461,7 +472,7 @@ impl ConjoiningClauses { Some(TypedValue::Keyword(ref kw)) => // Don't recurse: avoid needing to clone the keyword. schema.attribute_for_ident(kw) - .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.clone())) + .ok_or_else(|| EmptyBecause::InvalidAttributeIdent(kw.cloned())) .and_then(|attribute| self.table_for_attribute_and_value(attribute, value)), Some(v) => { // This pattern cannot match: the caller has bound a non-entity value to an @@ -590,4 +601,9 @@ fn associate_ident(schema: &mut Schema, i: NamespacedKeyword, e: Entid) { #[cfg(test)] fn add_attribute(schema: &mut Schema, e: Entid, a: Attribute) { schema.schema_map.insert(e, a); +} + +#[cfg(test)] +pub fn ident(ns: &str, name: &str) -> PatternNonValuePlace { + PatternNonValuePlace::Ident(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) } \ No newline at end of file diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index 0005e794..cede4ef1 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -8,6 +8,8 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. +use std::rc::Rc; + use mentat_core::{ Schema, TypedValue, @@ -21,6 +23,8 @@ use mentat_query::{ SrcVar, }; +use super::RcCloned; + use clauses::ConjoiningClauses; use types::{ @@ -94,11 +98,11 @@ impl ConjoiningClauses { PatternNonValuePlace::Entid(entid) => self.constrain_column_to_entity(col.clone(), DatomsColumn::Entity, entid), PatternNonValuePlace::Ident(ref ident) => { - if let Some(entid) = self.entid_for_ident(schema, ident) { + if let Some(entid) = self.entid_for_ident(schema, ident.as_ref()) { self.constrain_column_to_entity(col.clone(), DatomsColumn::Entity, entid) } else { // A resolution failure means we're done here. - self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.clone())); + self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.cloned())); return; } } @@ -123,12 +127,12 @@ impl ConjoiningClauses { self.constrain_attribute(col.clone(), entid); if !schema.is_attribute(entid) { - self.mark_known_empty(EmptyBecause::InvalidAttributeIdent(ident.clone())); + self.mark_known_empty(EmptyBecause::InvalidAttributeIdent(ident.cloned())); return; } } else { // A resolution failure means we're done here. - self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.clone())); + self.mark_known_empty(EmptyBecause::UnresolvedIdent(ident.cloned())); return; } } @@ -190,7 +194,7 @@ impl ConjoiningClauses { } else { // A resolution failure means we're done here: this attribute must have an // entity value. - self.mark_known_empty(EmptyBecause::UnresolvedIdent(kw.clone())); + self.mark_known_empty(EmptyBecause::UnresolvedIdent(kw.cloned())); return; } } else { @@ -237,7 +241,6 @@ impl ConjoiningClauses { }, } - } pub fn apply_pattern<'s, 'p>(&mut self, schema: &'s Schema, pattern: Pattern) { @@ -274,13 +277,13 @@ mod testing { use mentat_query::{ NamespacedKeyword, NonIntegerConstant, - PlainSymbol, Variable, }; use clauses::{ add_attribute, associate_ident, + ident, unit_type_set, }; @@ -299,8 +302,8 @@ mod testing { cc.apply_pattern(&schema, Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Constant(NonIntegerConstant::Boolean(true)), tx: PatternNonValuePlace::Placeholder, }); @@ -317,8 +320,8 @@ mod testing { cc.apply_pattern(&schema, Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Constant(NonIntegerConstant::Boolean(true)), tx: PatternNonValuePlace::Placeholder, }); @@ -337,11 +340,11 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); + let x = Variable::from_valid_name("?x"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Constant(NonIntegerConstant::Boolean(true)), tx: PatternNonValuePlace::Placeholder, }); @@ -377,7 +380,7 @@ mod testing { let mut cc = ConjoiningClauses::default(); let schema = Schema::default(); - let x = Variable(PlainSymbol::new("?x")); + let x = Variable::from_valid_name("?x"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), @@ -421,12 +424,12 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let a = Variable(PlainSymbol::new("?a")); - let v = Variable(PlainSymbol::new("?v")); + let x = Variable::from_valid_name("?x"); + let a = Variable::from_valid_name("?a"); + let v = Variable::from_valid_name("?v"); cc.input_variables.insert(a.clone()); - cc.value_bindings.insert(a.clone(), TypedValue::Keyword(NamespacedKeyword::new("foo", "bar"))); + cc.value_bindings.insert(a.clone(), TypedValue::Keyword(Rc::new(NamespacedKeyword::new("foo", "bar")))); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), @@ -459,10 +462,10 @@ mod testing { let mut cc = ConjoiningClauses::default(); let schema = Schema::default(); - let x = Variable(PlainSymbol::new("?x")); - let a = Variable(PlainSymbol::new("?a")); - let v = Variable(PlainSymbol::new("?v")); - let hello = TypedValue::String("hello".to_string()); + let x = Variable::from_valid_name("?x"); + let a = Variable::from_valid_name("?a"); + let v = Variable::from_valid_name("?v"); + let hello = TypedValue::typed_string("hello"); cc.input_variables.insert(a.clone()); cc.value_bindings.insert(a.clone(), hello.clone()); @@ -485,9 +488,9 @@ mod testing { let mut cc = ConjoiningClauses::default(); let schema = Schema::default(); - let x = Variable(PlainSymbol::new("?x")); - let a = Variable(PlainSymbol::new("?a")); - let v = Variable(PlainSymbol::new("?v")); + let x = Variable::from_valid_name("?x"); + let a = Variable::from_valid_name("?a"); + let v = Variable::from_valid_name("?v"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), @@ -517,12 +520,12 @@ mod testing { let mut cc = ConjoiningClauses::default(); let schema = Schema::default(); - let x = Variable(PlainSymbol::new("?x")); + let x = Variable::from_valid_name("?x"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), attribute: PatternNonValuePlace::Placeholder, - value: PatternValuePlace::Constant(NonIntegerConstant::Text("hello".to_string())), + value: PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new("hello".to_string()))), tx: PatternNonValuePlace::Placeholder, }); @@ -545,7 +548,7 @@ mod testing { // - datoms0.value_type_tag = string // TODO: implement expand_type_tags. assert_eq!(cc.wheres, vec![ - ColumnConstraint::Equals(d0_v, QueryValue::TypedValue(TypedValue::String("hello".to_string()))), + ColumnConstraint::Equals(d0_v, QueryValue::TypedValue(TypedValue::String(Rc::new("hello".to_string())))), ColumnConstraint::HasType("all_datoms00".to_string(), ValueType::String), ].into()); } @@ -566,19 +569,19 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "roz")), - value: PatternValuePlace::Constant(NonIntegerConstant::Text("idgoeshere".to_string())), + attribute: ident("foo", "roz"), + value: PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new("idgoeshere".to_string()))), tx: PatternNonValuePlace::Placeholder, }); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); @@ -617,7 +620,7 @@ mod testing { // - datoms1.e = datoms0.e assert_eq!(cc.wheres, vec![ ColumnConstraint::Equals(d0_a, QueryValue::Entid(98)), - ColumnConstraint::Equals(d0_v, QueryValue::TypedValue(TypedValue::String("idgoeshere".to_string()))), + ColumnConstraint::Equals(d0_v, QueryValue::TypedValue(TypedValue::typed_string("idgoeshere"))), ColumnConstraint::Equals(d1_a, QueryValue::Entid(99)), ColumnConstraint::Equals(d0_e, QueryValue::Column(d1_e)), ].into()); @@ -633,8 +636,8 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); let b: BTreeMap = vec![(y.clone(), TypedValue::Boolean(true))].into_iter().collect(); @@ -643,7 +646,7 @@ mod testing { cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); @@ -678,8 +681,8 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); let b: BTreeMap = vec![(y.clone(), TypedValue::Long(42))].into_iter().collect(); @@ -688,7 +691,7 @@ mod testing { cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); @@ -710,8 +713,8 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); let b: BTreeMap = vec![(y.clone(), TypedValue::Long(42))].into_iter().collect(); @@ -720,7 +723,7 @@ mod testing { cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); @@ -749,19 +752,19 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "roz")), + attribute: ident("foo", "roz"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), + attribute: ident("foo", "bar"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); @@ -786,9 +789,9 @@ mod testing { // [:find ?x :where // [?x ?y true] // [?z ?y ?x]] - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); - let z = Variable(PlainSymbol::new("?z")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); + let z = Variable::from_valid_name("?z"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), diff --git a/query-algebrizer/src/clauses/predicate.rs b/query-algebrizer/src/clauses/predicate.rs index c26d70f1..4242d9a0 100644 --- a/query-algebrizer/src/clauses/predicate.rs +++ b/query-algebrizer/src/clauses/predicate.rs @@ -88,7 +88,6 @@ mod testing { use super::*; use std::collections::HashSet; - use mentat_core::attribute::Unique; use mentat_core::{ Attribute, @@ -109,6 +108,7 @@ mod testing { use clauses::{ add_attribute, associate_ident, + ident, }; use types::{ @@ -117,7 +117,6 @@ mod testing { QueryValue, }; - #[test] /// Apply two patterns: a pattern and a numeric predicate. /// Verify that after application of the predicate we know that the value @@ -132,8 +131,8 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), @@ -148,7 +147,7 @@ mod testing { assert!(cc.apply_numeric_predicate(&schema, comp, Predicate { operator: op, args: vec![ - FnArg::Variable(Variable(PlainSymbol::new("?y"))), FnArg::EntidOrInteger(10), + FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10), ]}).is_ok()); assert!(!cc.is_known_empty); @@ -192,8 +191,8 @@ mod testing { ..Default::default() }); - let x = Variable(PlainSymbol::new("?x")); - let y = Variable(PlainSymbol::new("?y")); + let x = Variable::from_valid_name("?x"); + let y = Variable::from_valid_name("?y"); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), @@ -208,14 +207,14 @@ mod testing { assert!(cc.apply_numeric_predicate(&schema, comp, Predicate { operator: op, args: vec![ - FnArg::Variable(Variable(PlainSymbol::new("?y"))), FnArg::EntidOrInteger(10), + FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10), ]}).is_ok()); assert!(!cc.is_known_empty); cc.apply_pattern(&schema, Pattern { source: None, entity: PatternNonValuePlace::Variable(x.clone()), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "roz")), + attribute: ident("foo", "roz"), value: PatternValuePlace::Variable(y.clone()), tx: PatternNonValuePlace::Placeholder, }); diff --git a/query-algebrizer/src/clauses/resolve.rs b/query-algebrizer/src/clauses/resolve.rs index ff379740..aba0ba35 100644 --- a/query-algebrizer/src/clauses/resolve.rs +++ b/query-algebrizer/src/clauses/resolve.rs @@ -46,7 +46,7 @@ impl ConjoiningClauses { self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var))) + .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) }, // Can't be an entid. EntidOrInteger(i) => Ok(QueryValue::TypedValue(TypedValue::Long(i))), @@ -73,13 +73,13 @@ impl ConjoiningClauses { self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var))) + .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) }, EntidOrInteger(i) => Ok(QueryValue::PrimitiveLong(i)), Ident(_) => unimplemented!(), // TODO Constant(NonIntegerConstant::Boolean(val)) => Ok(QueryValue::TypedValue(TypedValue::Boolean(val))), Constant(NonIntegerConstant::Float(f)) => Ok(QueryValue::TypedValue(TypedValue::Double(f))), - Constant(NonIntegerConstant::Text(s)) => Ok(QueryValue::TypedValue(TypedValue::String(s.clone()))), + Constant(NonIntegerConstant::Text(s)) => Ok(QueryValue::TypedValue(TypedValue::typed_string(s.as_str()))), Constant(NonIntegerConstant::BigInteger(_)) => unimplemented!(), SrcVar(_) => unimplemented!(), } diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index 742f54a9..dff08556 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -12,7 +12,6 @@ extern crate mentat_query; use self::mentat_query::{ PlainSymbol, - Variable, }; error_chain! { @@ -31,9 +30,9 @@ error_chain! { display("invalid number of arguments to {}: expected {}, got {}.", name, expected, number) } - UnboundVariable(var: Variable) { + UnboundVariable(name: PlainSymbol) { description("unbound variable in function call") - display("unbound variable: {}", var.0) + display("unbound variable: {}", name) } NonNumericArgument(function: PlainSymbol, position: usize) { diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index 4eb56662..d10ff237 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -87,7 +87,6 @@ mod tests { Pattern, PatternNonValuePlace, PatternValuePlace, - PlainSymbol, UnifyVars, Variable, WhereClause, @@ -95,8 +94,14 @@ mod tests { use self::mentat_query_parser::parse_find_string; + use clauses::ident; + use super::validate_or_join; + fn value_ident(ns: &str, name: &str) -> PatternValuePlace { + PatternValuePlace::IdentOrKeyword(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) + } + /// Tests that the top-level form is a valid `or`, returning the clauses. fn valid_or_join(parsed: FindQuery, expected_unify: UnifyVars) -> Vec { let mut wheres = parsed.where_clauses.into_iter(); @@ -134,9 +139,9 @@ mod tests { left, OrWhereClause::Clause(WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?artist"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "type")), - value: PatternValuePlace::IdentOrKeyword(NamespacedKeyword::new("artist.type", "group")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?artist")), + attribute: ident("artist", "type"), + value: value_ident("artist.type", "group"), tx: PatternNonValuePlace::Placeholder, }))); assert_eq!( @@ -145,16 +150,16 @@ mod tests { vec![ WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?artist"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "type")), - value: PatternValuePlace::IdentOrKeyword(NamespacedKeyword::new("artist.type", "person")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?artist")), + attribute: ident("artist", "type"), + value: value_ident("artist.type", "person"), tx: PatternNonValuePlace::Placeholder, }), WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?artist"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "gender")), - value: PatternValuePlace::IdentOrKeyword(NamespacedKeyword::new("artist.gender", "female")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?artist")), + attribute: ident("artist", "gender"), + value: value_ident("artist.gender", "female"), tx: PatternNonValuePlace::Placeholder, }), ])); @@ -186,7 +191,7 @@ mod tests { (and [?artist :artist/type ?type] [?type :artist/role :artist.role/parody]))]"#; let parsed = parse_find_string(query).expect("expected successful parse"); - let clauses = valid_or_join(parsed, UnifyVars::Explicit(vec![Variable(PlainSymbol::new("?artist"))])); + let clauses = valid_or_join(parsed, UnifyVars::Explicit(vec![Variable::from_valid_name("?artist")])); // Let's do some detailed parse checks. let mut arms = clauses.into_iter(); @@ -196,9 +201,9 @@ mod tests { left, OrWhereClause::Clause(WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?artist"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "type")), - value: PatternValuePlace::IdentOrKeyword(NamespacedKeyword::new("artist.type", "group")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?artist")), + attribute: ident("artist", "type"), + value: value_ident("artist.type", "group"), tx: PatternNonValuePlace::Placeholder, }))); assert_eq!( @@ -207,16 +212,16 @@ mod tests { vec![ WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?artist"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "type")), - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?type"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?artist")), + attribute: ident("artist", "type"), + value: PatternValuePlace::Variable(Variable::from_valid_name("?type")), tx: PatternNonValuePlace::Placeholder, }), WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?type"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("artist", "role")), - value: PatternValuePlace::IdentOrKeyword(NamespacedKeyword::new("artist.role", "parody")), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?type")), + attribute: ident("artist", "role"), + value: value_ident("artist.role", "parody"), tx: PatternNonValuePlace::Placeholder, }), ])); diff --git a/query-parser/src/find.rs b/query-parser/src/find.rs index 56523d91..f3c1c22a 100644 --- a/query-parser/src/find.rs +++ b/query-parser/src/find.rs @@ -185,6 +185,8 @@ pub fn parse_find(expr: edn::Value) -> QueryParseResult { mod test_parse { extern crate edn; + use std::rc::Rc; + use self::edn::{NamespacedKeyword, PlainSymbol}; use self::edn::types::Value; use super::mentat_query::{ @@ -217,8 +219,8 @@ mod test_parse { let parsed = parse_find(input).unwrap(); if let FindSpec::FindRel(elems) = parsed.find_spec { assert_eq!(2, elems.len()); - assert_eq!(vec![Element::Variable(Variable(edn::PlainSymbol::new("?x"))), - Element::Variable(Variable(edn::PlainSymbol::new("?y")))], + assert_eq!(vec![Element::Variable(Variable::from_valid_name("?x")), + Element::Variable(Variable::from_valid_name("?y"))], elems); } else { panic!("Expected FindRel."); @@ -229,9 +231,9 @@ mod test_parse { vec![ WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?y"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: PatternNonValuePlace::Ident(Rc::new(NamespacedKeyword::new("foo", "bar"))), + value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), tx: PatternNonValuePlace::Placeholder, })]); } @@ -244,14 +246,14 @@ mod test_parse { vec![ WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?y"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: PatternNonValuePlace::Ident(Rc::new(NamespacedKeyword::new("foo", "bar"))), + value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), tx: PatternNonValuePlace::Placeholder, }), WhereClause::Pred(Predicate { operator: PlainSymbol::new("<"), - args: vec![FnArg::Variable(Variable(PlainSymbol::new("?y"))), + args: vec![FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10)], }), ]); diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs index fa9f8f9a..21305ca9 100644 --- a/query-parser/src/parse.rs +++ b/query-parser/src/parse.rs @@ -97,6 +97,7 @@ impl Query } } +// TODO: interning. def_value_satisfy_parser_fn!(Query, variable, Variable, Variable::from_value); def_value_satisfy_parser_fn!(Query, source_var, SrcVar, SrcVar::from_value); def_value_satisfy_parser_fn!(Query, predicate_fn, PredicateFn, PredicateFn::from_value); @@ -405,6 +406,8 @@ mod test { extern crate edn; extern crate mentat_query; + use std::rc::Rc; + use self::combine::Parser; use self::edn::OrderedFloat; use self::mentat_query::{ @@ -420,6 +423,18 @@ mod test { use super::*; + fn variable(x: edn::PlainSymbol) -> Variable { + Variable(Rc::new(x)) + } + + fn ident_kw(kw: edn::NamespacedKeyword) -> PatternNonValuePlace { + PatternNonValuePlace::Ident(Rc::new(kw)) + } + + fn ident(ns: &str, name: &str) -> PatternNonValuePlace { + ident_kw(edn::NamespacedKeyword::new(ns, name)) + } + #[test] fn test_pattern_mixed() { let e = edn::PlainSymbol::new("_"); @@ -433,9 +448,9 @@ mod test { assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { source: None, entity: PatternNonValuePlace::Placeholder, - attribute: PatternNonValuePlace::Ident(a), + attribute: ident_kw(a), value: PatternValuePlace::Constant(NonIntegerConstant::Float(v)), - tx: PatternNonValuePlace::Variable(Variable(tx)), + tx: PatternNonValuePlace::Variable(variable(tx)), })); } @@ -453,10 +468,10 @@ mod test { edn::Value::PlainSymbol(tx.clone())))]; assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { source: Some(SrcVar::NamedSrc("x".to_string())), - entity: PatternNonValuePlace::Variable(Variable(e)), - attribute: PatternNonValuePlace::Variable(Variable(a)), - value: PatternValuePlace::Variable(Variable(v)), - tx: PatternNonValuePlace::Variable(Variable(tx)), + entity: PatternNonValuePlace::Variable(variable(e)), + attribute: PatternNonValuePlace::Variable(variable(a)), + value: PatternValuePlace::Variable(variable(v)), + tx: PatternNonValuePlace::Variable(variable(tx)), })); } @@ -491,10 +506,10 @@ mod test { // switched places. assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(v)), - attribute: PatternNonValuePlace::Ident(edn::NamespacedKeyword::new("foo", "bar")), + entity: PatternNonValuePlace::Variable(variable(v)), + attribute: ident("foo", "bar"), value: PatternValuePlace::Placeholder, - tx: PatternNonValuePlace::Variable(Variable(tx)), + tx: PatternNonValuePlace::Variable(variable(tx)), })); } @@ -503,7 +518,7 @@ mod test { let e = edn::PlainSymbol::new("?e"); let input = [edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone())])]; assert_parses_to!(Where::rule_vars, input, - vec![Variable(e.clone())]); + vec![variable(e.clone())]); } #[test] @@ -524,9 +539,9 @@ mod test { clauses: vec![OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(e)), - attribute: PatternNonValuePlace::Variable(Variable(a)), - value: PatternValuePlace::Variable(Variable(v)), + entity: PatternNonValuePlace::Variable(variable(e)), + attribute: PatternNonValuePlace::Variable(variable(a)), + value: PatternValuePlace::Variable(variable(v)), tx: PatternNonValuePlace::Placeholder, }))], })); @@ -547,13 +562,13 @@ mod test { assert_parses_to!(Where::or_join_clause, input, WhereClause::OrJoin( OrJoin { - unify_vars: UnifyVars::Explicit(vec![Variable(e.clone())]), + unify_vars: UnifyVars::Explicit(vec![variable(e.clone())]), clauses: vec![OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(e)), - attribute: PatternNonValuePlace::Variable(Variable(a)), - value: PatternValuePlace::Variable(Variable(v)), + entity: PatternNonValuePlace::Variable(variable(e)), + attribute: PatternNonValuePlace::Variable(variable(a)), + value: PatternValuePlace::Variable(variable(v)), tx: PatternNonValuePlace::Placeholder, }))], })); @@ -563,7 +578,7 @@ mod test { fn test_find_sp_variable() { let sym = edn::PlainSymbol::new("?x"); let input = [edn::Value::PlainSymbol(sym.clone())]; - assert_parses_to!(Query::variable, input, Variable(sym)); + assert_parses_to!(Query::variable, input, variable(sym)); } #[test] @@ -573,7 +588,7 @@ mod test { let input = [edn::Value::PlainSymbol(sym.clone()), edn::Value::PlainSymbol(period.clone())]; assert_parses_to!(Find::find_scalar, input, - FindSpec::FindScalar(Element::Variable(Variable(sym)))); + FindSpec::FindScalar(Element::Variable(variable(sym)))); } #[test] @@ -584,7 +599,7 @@ mod test { edn::Value::PlainSymbol(period.clone())])]; assert_parses_to!(Find::find_coll, input, - FindSpec::FindColl(Element::Variable(Variable(sym)))); + FindSpec::FindColl(Element::Variable(variable(sym)))); } #[test] @@ -594,8 +609,8 @@ mod test { let input = [edn::Value::PlainSymbol(vx.clone()), edn::Value::PlainSymbol(vy.clone())]; assert_parses_to!(Find::find_rel, input, - FindSpec::FindRel(vec![Element::Variable(Variable(vx)), - Element::Variable(Variable(vy))])); + FindSpec::FindRel(vec![Element::Variable(variable(vx)), + Element::Variable(variable(vy))])); } #[test] @@ -606,8 +621,8 @@ mod test { edn::Value::PlainSymbol(vy.clone())])]; assert_parses_to!(Find::find_tuple, input, - FindSpec::FindTuple(vec![Element::Variable(Variable(vx)), - Element::Variable(Variable(vy))])); + FindSpec::FindTuple(vec![Element::Variable(variable(vx)), + Element::Variable(variable(vy))])); } #[test] @@ -624,15 +639,15 @@ mod test { edn::Value::PlainSymbol(ellipsis.clone())])]; let rel = [edn::Value::PlainSymbol(vx.clone()), edn::Value::PlainSymbol(vy.clone())]; - assert_eq!(FindSpec::FindScalar(Element::Variable(Variable(vx.clone()))), + assert_eq!(FindSpec::FindScalar(Element::Variable(variable(vx.clone()))), find_seq_to_find_spec(&scalar).unwrap()); - assert_eq!(FindSpec::FindTuple(vec![Element::Variable(Variable(vx.clone())), - Element::Variable(Variable(vy.clone()))]), + assert_eq!(FindSpec::FindTuple(vec![Element::Variable(variable(vx.clone())), + Element::Variable(variable(vy.clone()))]), find_seq_to_find_spec(&tuple).unwrap()); - assert_eq!(FindSpec::FindColl(Element::Variable(Variable(vx.clone()))), + assert_eq!(FindSpec::FindColl(Element::Variable(variable(vx.clone()))), find_seq_to_find_spec(&coll).unwrap()); - assert_eq!(FindSpec::FindRel(vec![Element::Variable(Variable(vx.clone())), - Element::Variable(Variable(vy.clone()))]), + assert_eq!(FindSpec::FindRel(vec![Element::Variable(variable(vx.clone())), + Element::Variable(variable(vy.clone()))]), find_seq_to_find_spec(&rel).unwrap()); } } diff --git a/query-parser/tests/find_tests.rs b/query-parser/tests/find_tests.rs index bfefe20f..dba420ed 100644 --- a/query-parser/tests/find_tests.rs +++ b/query-parser/tests/find_tests.rs @@ -45,18 +45,18 @@ fn can_parse_predicates() { let p = parse_find_string(s).unwrap(); assert_eq!(p.find_spec, - FindSpec::FindColl(Element::Variable(Variable(PlainSymbol::new("?x"))))); + FindSpec::FindColl(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?y"))), + value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), tx: PatternNonValuePlace::Placeholder, }), WhereClause::Pred(Predicate { operator: PlainSymbol::new("<"), args: vec![ - FnArg::Variable(Variable(PlainSymbol::new("?y"))), FnArg::EntidOrInteger(10), + FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10), ]}), ]); } @@ -67,7 +67,7 @@ fn can_parse_simple_or() { let p = parse_find_string(s).unwrap(); assert_eq!(p.find_spec, - FindSpec::FindScalar(Element::Variable(Variable(PlainSymbol::new("?x"))))); + FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin { @@ -76,7 +76,7 @@ fn can_parse_simple_or() { OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(10), tx: PatternNonValuePlace::Placeholder, @@ -84,7 +84,7 @@ fn can_parse_simple_or() { OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(15), tx: PatternNonValuePlace::Placeholder, @@ -100,16 +100,16 @@ fn can_parse_unit_or_join() { let p = parse_find_string(s).unwrap(); assert_eq!(p.find_spec, - FindSpec::FindScalar(Element::Variable(Variable(PlainSymbol::new("?x"))))); + FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin { - unify_vars: UnifyVars::Explicit(vec![Variable(PlainSymbol::new("?x"))]), + unify_vars: UnifyVars::Explicit(vec![Variable::from_valid_name("?x")]), clauses: vec![ OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(15), tx: PatternNonValuePlace::Placeholder, @@ -125,16 +125,16 @@ fn can_parse_simple_or_join() { let p = parse_find_string(s).unwrap(); assert_eq!(p.find_spec, - FindSpec::FindScalar(Element::Variable(Variable(PlainSymbol::new("?x"))))); + FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin { - unify_vars: UnifyVars::Explicit(vec![Variable(PlainSymbol::new("?x"))]), + unify_vars: UnifyVars::Explicit(vec![Variable::from_valid_name("?x")]), clauses: vec![ OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(10), tx: PatternNonValuePlace::Placeholder, @@ -142,7 +142,7 @@ fn can_parse_simple_or_join() { OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(15), tx: PatternNonValuePlace::Placeholder, @@ -152,13 +152,18 @@ fn can_parse_simple_or_join() { ]); } +#[cfg(test)] +fn ident(ns: &str, name: &str) -> PatternNonValuePlace { + PatternNonValuePlace::Ident(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) +} + #[test] fn can_parse_simple_or_and_join() { let s = "[:find ?x . :where (or [?x _ 10] (and (or [?x :foo/bar ?y] [?x :foo/baz ?y]) [(< ?y 1)]))]"; let p = parse_find_string(s).unwrap(); assert_eq!(p.find_spec, - FindSpec::FindScalar(Element::Variable(Variable(PlainSymbol::new("?x"))))); + FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin { @@ -167,7 +172,7 @@ fn can_parse_simple_or_and_join() { OrWhereClause::Clause( WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), attribute: PatternNonValuePlace::Placeholder, value: PatternValuePlace::EntidOrInteger(10), tx: PatternNonValuePlace::Placeholder, @@ -179,27 +184,27 @@ fn can_parse_simple_or_and_join() { clauses: vec![ OrWhereClause::Clause(WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "bar")), - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?y"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: ident("foo", "bar"), + value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), tx: PatternNonValuePlace::Placeholder, })), OrWhereClause::Clause(WhereClause::Pattern(Pattern { source: None, - entity: PatternNonValuePlace::Variable(Variable(PlainSymbol::new("?x"))), - attribute: PatternNonValuePlace::Ident(NamespacedKeyword::new("foo", "baz")), - value: PatternValuePlace::Variable(Variable(PlainSymbol::new("?y"))), + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + attribute: ident("foo", "baz"), + value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), tx: PatternNonValuePlace::Placeholder, })), ], }), WhereClause::Pred(Predicate { operator: PlainSymbol::new("<"), args: vec![ - FnArg::Variable(Variable(PlainSymbol::new("?y"))), FnArg::EntidOrInteger(1), + FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(1), ]}), ], ) ], }), ]); -} \ No newline at end of file +} diff --git a/query-projector/src/lib.rs b/query-projector/src/lib.rs index 9ec181cd..f1d94596 100644 --- a/query-projector/src/lib.rs +++ b/query-projector/src/lib.rs @@ -20,7 +20,6 @@ extern crate mentat_query_sql; extern crate mentat_sql; use std::iter; - use rusqlite::{ Row, Rows, @@ -38,7 +37,6 @@ use mentat_db::{ use mentat_query::{ Element, FindSpec, - PlainSymbol, Variable, }; @@ -172,13 +170,11 @@ impl TypedIndex { } fn column_name(var: &Variable) -> Name { - let &Variable(PlainSymbol(ref s)) = var; - s.clone() + var.to_string() } fn value_type_tag_name(var: &Variable) -> Name { - let &Variable(PlainSymbol(ref s)) = var; - format!("{}_value_type_tag", s) + format!("{}_value_type_tag", var.as_str()) } /// Walk an iterator of `Element`s, collecting projector templates and columns. diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index bcb0710a..29ad6c75 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -15,6 +15,8 @@ extern crate mentat_query_parser; extern crate mentat_query_translator; extern crate mentat_sql; +use std::rc::Rc; + use mentat_query::NamespacedKeyword; use mentat_core::{ @@ -59,6 +61,10 @@ fn prepopulated_schema() -> Schema { schema } +fn make_arg(name: &'static str, value: &'static str) -> (String, Rc) { + (name.to_string(), Rc::new(value.to_string())) +} + #[test] fn test_scalar() { let schema = prepopulated_schema(); @@ -66,7 +72,7 @@ fn test_scalar() { let input = r#"[:find ?x . :where [?x :foo/bar "yyy"]]"#; let SQLQuery { sql, args } = translate(&schema, input, None); assert_eq!(sql, "SELECT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 LIMIT 1"); - assert_eq!(args, vec![("$v0".to_string(), "yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "yyy")]); } #[test] @@ -76,7 +82,7 @@ fn test_tuple() { let input = r#"[:find [?x] :where [?x :foo/bar "yyy"]]"#; let SQLQuery { sql, args } = translate(&schema, input, None); assert_eq!(sql, "SELECT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 LIMIT 1"); - assert_eq!(args, vec![("$v0".to_string(), "yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "yyy")]); } #[test] @@ -86,7 +92,7 @@ fn test_coll() { let input = r#"[:find [?x ...] :where [?x :foo/bar "yyy"]]"#; let SQLQuery { sql, args } = translate(&schema, input, None); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0"); - assert_eq!(args, vec![("$v0".to_string(), "yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "yyy")]); } #[test] @@ -96,7 +102,7 @@ fn test_rel() { let input = r#"[:find ?x :where [?x :foo/bar "yyy"]]"#; let SQLQuery { sql, args } = translate(&schema, input, None); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0"); - assert_eq!(args, vec![("$v0".to_string(), "yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "yyy")]); } #[test] @@ -106,7 +112,7 @@ fn test_limit() { let input = r#"[:find ?x :where [?x :foo/bar "yyy"]]"#; let SQLQuery { sql, args } = translate(&schema, input, 5); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 LIMIT 5"); - assert_eq!(args, vec![("$v0".to_string(), "yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "yyy")]); } #[test] @@ -118,7 +124,7 @@ fn test_unknown_attribute_keyword_value() { // Only match keywords, not strings: tag = 13. assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.v = $v0 AND `datoms00`.value_type_tag = 13"); - assert_eq!(args, vec![("$v0".to_string(), ":ab/yyy".to_string())]); + assert_eq!(args, vec![make_arg("$v0", ":ab/yyy")]); } #[test] @@ -131,7 +137,7 @@ fn test_unknown_attribute_string_value() { // We expect all_datoms because we're querying for a string. Magic, that. // We don't want keywords etc., so tag = 10. assert_eq!(sql, "SELECT DISTINCT `all_datoms00`.e AS `?x` FROM `all_datoms` AS `all_datoms00` WHERE `all_datoms00`.v = $v0 AND `all_datoms00`.value_type_tag = 10"); - assert_eq!(args, vec![("$v0".to_string(), "horses".to_string())]); + assert_eq!(args, vec![make_arg("$v0", "horses")]); } #[test] diff --git a/query/src/lib.rs b/query/src/lib.rs index 40ddb06b..23fcdf3c 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -35,6 +35,7 @@ extern crate mentat_core; use std::collections::BTreeSet; use std::fmt; +use std::rc::Rc; use edn::{BigInt, OrderedFloat}; pub use edn::{NamespacedKeyword, PlainSymbol}; use mentat_core::TypedValue; @@ -42,26 +43,26 @@ use mentat_core::TypedValue; pub type SrcVarName = String; // Do not include the required syntactic '$'. #[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct Variable(pub PlainSymbol); +pub struct Variable(pub Rc); impl Variable { pub fn as_str(&self) -> &str { - (self.0).0.as_str() + self.0.as_ref().0.as_str() } pub fn to_string(&self) -> String { - (self.0).0.clone() + self.0.as_ref().0.clone() } pub fn name(&self) -> PlainSymbol { - self.0.clone() + self.0.as_ref().clone() } /// Return a new `Variable`, assuming that the provided string is a valid name. pub fn from_valid_name(name: &str) -> Variable { let s = PlainSymbol::new(name); assert!(s.is_var_symbol()); - Variable(s) + Variable(Rc::new(s)) } } @@ -71,6 +72,7 @@ pub trait FromValue { /// If the provided EDN value is a PlainSymbol beginning with '?', return /// it wrapped in a Variable. If not, return None. +/// TODO: intern strings. #398. impl FromValue for Variable { fn from_value(v: &edn::Value) -> Option { if let edn::Value::PlainSymbol(ref s) = *v { @@ -82,13 +84,22 @@ impl FromValue for Variable { } impl Variable { - pub fn from_symbol(sym: &PlainSymbol) -> Option { + pub fn from_rc(sym: Rc) -> Option { if sym.is_var_symbol() { Some(Variable(sym.clone())) } else { None } } + + /// TODO: intern strings. #398. + pub fn from_symbol(sym: &PlainSymbol) -> Option { + if sym.is_var_symbol() { + Some(Variable(Rc::new(sym.clone()))) + } else { + None + } + } } impl fmt::Debug for Variable { @@ -149,7 +160,7 @@ pub enum NonIntegerConstant { Boolean(bool), BigInteger(BigInt), Float(OrderedFloat), - Text(String), + Text(Rc), } impl NonIntegerConstant { @@ -197,7 +208,7 @@ pub enum PatternNonValuePlace { Placeholder, Variable(Variable), Entid(i64), // Will always be +ve. See #190. - Ident(NamespacedKeyword), + Ident(Rc), } impl PatternNonValuePlace { @@ -240,7 +251,7 @@ impl FromValue for PatternNonValuePlace { } }, &edn::Value::NamespacedKeyword(ref x) => - Some(PatternNonValuePlace::Ident(x.clone())), + Some(PatternNonValuePlace::Ident(Rc::new(x.clone()))), _ => None, } } @@ -248,7 +259,7 @@ impl FromValue for PatternNonValuePlace { #[derive(Clone, Debug, Eq, PartialEq)] pub enum IdentOrEntid { - Ident(NamespacedKeyword), + Ident(Rc), Entid(i64), } @@ -260,7 +271,7 @@ pub enum PatternValuePlace { Placeholder, Variable(Variable), EntidOrInteger(i64), - IdentOrKeyword(NamespacedKeyword), + IdentOrKeyword(Rc), Constant(NonIntegerConstant), } @@ -271,10 +282,10 @@ impl FromValue for PatternValuePlace { Some(PatternValuePlace::EntidOrInteger(x)), &edn::Value::PlainSymbol(ref x) if x.0.as_str() == "_" => Some(PatternValuePlace::Placeholder), - &edn::Value::PlainSymbol(ref x) if x.is_var_symbol() => - Some(PatternValuePlace::Variable(Variable(x.clone()))), + &edn::Value::PlainSymbol(ref x) => + Variable::from_symbol(x).map(PatternValuePlace::Variable), &edn::Value::NamespacedKeyword(ref x) => - Some(PatternValuePlace::IdentOrKeyword(x.clone())), + Some(PatternValuePlace::IdentOrKeyword(Rc::new(x.clone()))), &edn::Value::Boolean(x) => Some(PatternValuePlace::Constant(NonIntegerConstant::Boolean(x))), &edn::Value::Float(x) => @@ -282,7 +293,8 @@ impl FromValue for PatternValuePlace { &edn::Value::BigInteger(ref x) => Some(PatternValuePlace::Constant(NonIntegerConstant::BigInteger(x.clone()))), &edn::Value::Text(ref x) => - Some(PatternValuePlace::Constant(NonIntegerConstant::Text(x.clone()))), + // TODO: intern strings. #398. + Some(PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new(x.clone())))), _ => None, } } @@ -359,14 +371,15 @@ pub enum Element { /// ```rust /// # extern crate edn; /// # extern crate mentat_query; +/// # use std::rc::Rc; /// # use edn::PlainSymbol; /// # use mentat_query::{Element, FindSpec, Variable}; /// /// # fn main() { /// /// let elements = vec![ -/// Element::Variable(Variable(PlainSymbol("?foo".to_string()))), -/// Element::Variable(Variable(PlainSymbol("?bar".to_string()))), +/// Element::Variable(Variable::from_valid_name("?foo")), +/// Element::Variable(Variable::from_valid_name("?bar")), /// ]; /// let rel = FindSpec::FindRel(elements); /// @@ -471,7 +484,7 @@ impl Pattern { return Some(Pattern { source: src, entity: v_e, - attribute: PatternNonValuePlace::Ident(k.to_reversed()), + attribute: PatternNonValuePlace::Ident(Rc::new(k.to_reversed())), value: e_v, tx: tx, }); @@ -672,4 +685,4 @@ impl ContainsVariables for Pattern { acc_ref(acc, v) } } -} +} \ No newline at end of file diff --git a/sql/src/lib.rs b/sql/src/lib.rs index 35dfface..a1c1f7b8 100644 --- a/sql/src/lib.rs +++ b/sql/src/lib.rs @@ -13,6 +13,8 @@ extern crate error_chain; extern crate ordered_float; extern crate mentat_core; +use std::rc::Rc; + use ordered_float::OrderedFloat; use mentat_core::TypedValue; @@ -45,7 +47,7 @@ pub struct SQLQuery { pub sql: String, /// These will eventually perhaps be rusqlite `ToSql` instances. - pub args: Vec<(String, String)>, + pub args: Vec<(String, Rc)>, } /// Gratefully based on Diesel's QueryBuilder trait: @@ -86,7 +88,7 @@ pub struct SQLiteQueryBuilder { arg_prefix: String, arg_counter: i64, - args: Vec<(String, String)>, + args: Vec<(String, Rc)>, } impl SQLiteQueryBuilder { @@ -103,7 +105,7 @@ impl SQLiteQueryBuilder { } } - fn push_static_arg(&mut self, val: String) { + fn push_static_arg(&mut self, val: Rc) { let arg = format!("{}{}", self.arg_prefix, self.arg_counter); self.arg_counter = self.arg_counter + 1; self.push_named_arg(arg.as_str()); @@ -134,8 +136,11 @@ impl QueryBuilder for SQLiteQueryBuilder { &Boolean(v) => self.push_sql(if v { "1" } else { "0" }), &Long(v) => self.push_sql(v.to_string().as_str()), &Double(OrderedFloat(v)) => self.push_sql(v.to_string().as_str()), + + // These are both `Rc`. We can just clone an `Rc`, but we + // must make a new single `String`, wrapped in an `Rc`, for keywords. &String(ref s) => self.push_static_arg(s.clone()), - &Keyword(ref s) => self.push_static_arg(s.to_string()), + &Keyword(ref s) => self.push_static_arg(Rc::new(s.as_ref().to_string())), } Ok(()) } @@ -183,14 +188,14 @@ mod tests { s.push_sql(" WHERE "); s.push_identifier("bar").unwrap(); s.push_sql(" = "); - s.push_static_arg("frobnicate".to_string()); + s.push_static_arg(Rc::new("frobnicate".to_string())); s.push_sql(" OR "); - s.push_static_arg("swoogle".to_string()); + s.push_static_arg(Rc::new("swoogle".to_string())); let q = s.finish(); assert_eq!(q.sql.as_str(), "SELECT `foo` WHERE `bar` = $v0 OR $v1"); assert_eq!(q.args, - vec![("$v0".to_string(), "frobnicate".to_string()), - ("$v1".to_string(), "swoogle".to_string())]); + vec![("$v0".to_string(), Rc::new("frobnicate".to_string())), + ("$v1".to_string(), Rc::new("swoogle".to_string()))]); } } diff --git a/src/query.rs b/src/query.rs index 5f8e005a..d1dbd0e2 100644 --- a/src/query.rs +++ b/src/query.rs @@ -84,7 +84,7 @@ pub fn q_once<'sqlite, 'schema, 'query, T, U> } else { let refs: Vec<(&str, &ToSql)> = args.iter() - .map(|&(ref k, ref v)| (k.as_str(), v as &ToSql)) + .map(|&(ref k, ref v)| (k.as_str(), v.as_ref() as &ToSql)) .collect(); statement.query_named(refs.as_slice())? }; diff --git a/tests/query.rs b/tests/query.rs index fe057c77..8ad3b582 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -92,11 +92,11 @@ fn test_scalar() { assert_eq!(1, results.len()); - if let QueryResults::Scalar(Some(TypedValue::Keyword(ref kw))) = results { + if let QueryResults::Scalar(Some(TypedValue::Keyword(ref rc))) = results { // Should be '24'. - assert_eq!(&NamespacedKeyword::new("db.type", "keyword"), kw); + assert_eq!(&NamespacedKeyword::new("db.type", "keyword"), rc.as_ref()); assert_eq!(24, - db.schema.get_entid(kw).unwrap()); + db.schema.get_entid(rc).unwrap()); } else { panic!("Expected scalar."); }