diff --git a/core/src/lib.rs b/core/src/lib.rs index a2095964..a2f54d67 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -55,12 +55,15 @@ mod value_type_set; mod sql_types; pub use types::{ + Cloned, Entid, + FromRc, KnownEntid, TypedValue, Binding, ValueType, ValueTypeTag, + ValueRc, now, }; diff --git a/core/src/types.rs b/core/src/types.rs index f6121bd0..31b5723c 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -8,9 +8,20 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -use std::fmt; -use std::rc::Rc; +use ::std::ffi::{ + CString, +}; +use ::std::os::raw::c_char; +use ::std::rc::{ + Rc, +}; + +use ::std::sync::{ + Arc, +}; + +use std::fmt; use ::enum_set::EnumSet; use ::ordered_float::OrderedFloat; @@ -35,6 +46,83 @@ use ::edn::{ use values; +pub trait FromRc { + fn from_rc(val: Rc) -> Self; + fn from_arc(val: Arc) -> Self; +} + +impl FromRc for Rc where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + val.clone() + } + + fn from_arc(val: Arc) -> Self { + match ::std::sync::Arc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } +} + +impl FromRc for Arc where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + match ::std::rc::Rc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } + + fn from_arc(val: Arc) -> Self { + val.clone() + } +} + +impl FromRc for Box where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + match ::std::rc::Rc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } + + fn from_arc(val: Arc) -> Self { + match ::std::sync::Arc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } +} + +// We do this a lot for errors. +pub trait Cloned { + fn cloned(&self) -> T; +} + +impl Cloned for Rc where T: Sized + Clone { + fn cloned(&self) -> T { + (*self.as_ref()).clone() + } +} + +impl Cloned for Arc where T: Sized + Clone { + fn cloned(&self) -> T { + (*self.as_ref()).clone() + } +} + +impl Cloned for Box where T: Sized + Clone { + fn cloned(&self) -> T { + self.as_ref().clone() + } +} + +/// +/// This type alias exists to allow us to use different boxing mechanisms for values. +/// This type must implement `FromRc` and `Cloned`, and a `From` implementation must exist for +/// `TypedValue`. +/// +pub type ValueRc = Rc; + /// Represents one entid in the entid space. /// /// Per https://www.sqlite.org/datatype3.html (see also http://stackoverflow.com/a/8499544), SQLite @@ -53,12 +141,6 @@ impl From for Entid { } } -impl From for Binding { - fn from(k: KnownEntid) -> Binding { - Binding::Scalar(TypedValue::Ref(k.0)) - } -} - impl From for TypedValue { fn from(k: KnownEntid) -> TypedValue { TypedValue::Ref(k.0) @@ -177,8 +259,8 @@ pub enum TypedValue { Double(OrderedFloat), Instant(DateTime), // Use `into()` to ensure truncation. // TODO: &str throughout? - String(Rc), - Keyword(Rc), + String(ValueRc), + Keyword(ValueRc), Uuid(Uuid), // It's only 128 bits, so this should be acceptable to clone. } @@ -197,26 +279,13 @@ pub enum TypedValue { #[derive(Clone, Debug, Eq, PartialEq)] pub enum Binding { Scalar(TypedValue), - Vec(Rc>), - Map(Rc), + Vec(ValueRc>), + Map(ValueRc), } -impl From for Binding { - fn from(src: TypedValue) -> Self { - Binding::Scalar(src) - } -} - - -impl<'a> From<&'a str> for Binding { - fn from(value: &'a str) -> Binding { - Binding::Scalar(TypedValue::String(Rc::new(value.to_string()))) - } -} - -impl From for Binding { - fn from(value: String) -> Binding { - Binding::Scalar(TypedValue::String(Rc::new(value))) +impl From for Binding where T: Into { + fn from(value: T) -> Binding { + Binding::Scalar(value.into()) } } @@ -239,8 +308,8 @@ impl Binding { /// /// We entirely support the former, and partially support the latter -- you can alias /// using a different keyword only. -#[derive(Debug, Eq, PartialEq)] -pub struct StructuredMap(IndexMap, Binding>); +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct StructuredMap(IndexMap, Binding>); impl Binding { /// Returns true if the provided type is `Some` and matches this value's type, or if the @@ -293,17 +362,17 @@ impl TypedValue { } /// Construct a new `TypedValue::Keyword` instance by cloning the provided - /// values and wrapping them in a new `Rc`. This is expensive, so this might + /// values and wrapping them in a new `ValueRc`. This is expensive, so this might /// be best limited to tests. pub fn typed_ns_keyword(ns: &str, name: &str) -> TypedValue { - TypedValue::Keyword(Rc::new(NamespacedKeyword::new(ns, name))) + NamespacedKeyword::new(ns, name).into() } /// Construct a new `TypedValue::String` instance by cloning the provided - /// value and wrapping it in a new `Rc`. This is expensive, so this might + /// value and wrapping it in a new `ValueRc`. This is expensive, so this might /// be best limited to tests. pub fn typed_string(s: &str) -> TypedValue { - TypedValue::String(Rc::new(s.to_string())) + s.into() } pub fn current_instant() -> TypedValue { @@ -363,19 +432,49 @@ impl From for TypedValue { impl<'a> From<&'a str> for TypedValue { fn from(value: &'a str) -> TypedValue { - TypedValue::String(Rc::new(value.to_string())) + TypedValue::String(ValueRc::new(value.to_string())) + } +} + +impl From> for TypedValue { + fn from(value: Arc) -> TypedValue { + TypedValue::String(ValueRc::from_arc(value)) + } +} + +impl From> for TypedValue { + fn from(value: Rc) -> TypedValue { + TypedValue::String(ValueRc::from_rc(value)) + } +} + +impl From> for TypedValue { + fn from(value: Box) -> TypedValue { + TypedValue::String(ValueRc::new(*value)) } } impl From for TypedValue { fn from(value: String) -> TypedValue { - TypedValue::String(Rc::new(value)) + TypedValue::String(ValueRc::new(value)) + } +} + +impl From> for TypedValue { + fn from(value: Arc) -> TypedValue { + TypedValue::Keyword(ValueRc::from_arc(value)) + } +} + +impl From> for TypedValue { + fn from(value: Rc) -> TypedValue { + TypedValue::Keyword(ValueRc::from_rc(value)) } } impl From for TypedValue { fn from(value: NamespacedKeyword) -> TypedValue { - TypedValue::Keyword(Rc::new(value)) + TypedValue::Keyword(ValueRc::new(value)) } } @@ -412,7 +511,7 @@ impl TypedValue { } } - pub fn into_kw(self) -> Option> { + pub fn into_kw(self) -> Option> { match self { TypedValue::Keyword(v) => Some(v), _ => None, @@ -454,13 +553,61 @@ impl TypedValue { } } - pub fn into_string(self) -> Option> { + pub fn into_string(self) -> Option> { match self { TypedValue::String(v) => Some(v), _ => None, } } + pub fn into_c_string(self) -> Option<*mut c_char> { + match self { + TypedValue::String(v) => { + // Get an independent copy of the string. + let s: String = v.cloned(); + + // Make a CString out of the new bytes. + let c: CString = CString::new(s).expect("String conversion failed!"); + + // Return a C-owned pointer. + Some(c.into_raw()) + }, + _ => None, + } + } + + pub fn into_kw_c_string(self) -> Option<*mut c_char> { + match self { + TypedValue::Keyword(v) => { + // Get an independent copy of the string. + let s: String = v.to_string(); + + // Make a CString out of the new bytes. + let c: CString = CString::new(s).expect("String conversion failed!"); + + // Return a C-owned pointer. + Some(c.into_raw()) + }, + _ => None, + } + } + + pub fn into_uuid_c_string(self) -> Option<*mut c_char> { + match self { + TypedValue::Uuid(v) => { + // Get an independent copy of the string. + let s: String = v.hyphenated().to_string(); + + // Make a CString out of the new bytes. + let c: CString = CString::new(s).expect("String conversion failed!"); + + // Return a C-owned pointer. + Some(c.into_raw()) + }, + _ => None, + } + } + pub fn into_uuid(self) -> Option { match self { TypedValue::Uuid(v) => Some(v), @@ -491,7 +638,7 @@ impl Binding { } } - pub fn into_kw(self) -> Option> { + pub fn into_kw(self) -> Option> { match self { Binding::Scalar(TypedValue::Keyword(v)) => Some(v), _ => None, @@ -533,7 +680,7 @@ impl Binding { } } - pub fn into_string(self) -> Option> { + pub fn into_string(self) -> Option> { match self { Binding::Scalar(TypedValue::String(v)) => Some(v), _ => None, diff --git a/db/src/cache.rs b/db/src/cache.rs index c1e1cb4c..92558faf 100644 --- a/db/src/cache.rs +++ b/db/src/cache.rs @@ -68,8 +68,6 @@ use std::iter::{ use std::mem; -use std::rc::Rc; - use std::sync::Arc; use std::iter::Peekable; @@ -85,6 +83,7 @@ use mentat_core::{ Schema, TypedValue, UpdateableCache, + ValueRc, }; use mentat_core::util::{ @@ -190,7 +189,7 @@ pub type Aev = (Entid, Entid, TypedValue); pub struct AevFactory { // Our own simple string-interning system. - strings: HashSet>, + strings: HashSet>, } impl AevFactory { diff --git a/db/src/db.rs b/db/src/db.rs index a0ed1ffb..fa86fa22 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -19,7 +19,6 @@ 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; @@ -51,7 +50,9 @@ use mentat_core::{ TypedValue, ToMicros, ValueType, + ValueRc, }; + use errors::{ErrorKind, Result, ResultExt}; use metadata; use schema::{ @@ -389,7 +390,7 @@ 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(Rc::new(x))), + (10, rusqlite::types::Value::Text(x)) => Ok(x.into()), (11, rusqlite::types::Value::Blob(x)) => { let u = Uuid::from_bytes(x.as_slice()); if u.is_err() { @@ -400,7 +401,7 @@ impl TypedSQLValue for TypedValue { Ok(TypedValue::Uuid(u.unwrap())) }, (13, rusqlite::types::Value::Text(x)) => { - to_namespaced_keyword(&x).map(|k| TypedValue::Keyword(Rc::new(k))) + to_namespaced_keyword(&x).map(|k| k.into()) }, (_, value) => bail!(ErrorKind::BadSQLValuePair(value, value_type_tag)), } @@ -420,8 +421,8 @@ impl TypedSQLValue for TypedValue { &Value::Integer(x) => Some(TypedValue::Long(x)), &Value::Uuid(x) => Some(TypedValue::Uuid(x)), &Value::Float(ref x) => Some(TypedValue::Double(x.clone())), - &Value::Text(ref x) => Some(TypedValue::String(Rc::new(x.clone()))), - &Value::NamespacedKeyword(ref x) => Some(TypedValue::Keyword(Rc::new(x.clone()))), + &Value::Text(ref x) => Some(x.clone().into()), + &Value::NamespacedKeyword(ref x) => Some(x.clone().into()), _ => None } } @@ -872,7 +873,7 @@ impl MentatStoring for rusqlite::Connection { let chunks: itertools::IntoChunks<_> = entities.into_iter().chunks(max_vars / bindings_per_statement); // From string to (searchid, value_type_tag). - let mut seen: HashMap, (i64, i32)> = HashMap::with_capacity(entities.len()); + let mut seen: HashMap, (i64, i32)> = HashMap::with_capacity(entities.len()); // We'd like to flat_map here, but it's not obvious how to flat_map across Result. let results: Result> = chunks.into_iter().map(|chunk| -> Result<()> { diff --git a/db/src/debug.rs b/db/src/debug.rs index b231796a..89991445 100644 --- a/db/src/debug.rs +++ b/db/src/debug.rs @@ -14,7 +14,6 @@ use std::borrow::Borrow; use std::io::{Write}; -use std::rc::Rc; use itertools::Itertools; use rusqlite; @@ -112,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(|i| TypedValue::Keyword(Rc::new(i))).unwrap_or(TypedValue::Ref(e)) + schema.get_ident(e).cloned().map(|i| i.into()).unwrap_or(TypedValue::Ref(e)) } else { self } diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index ff52d649..50ab0096 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -52,7 +52,6 @@ pub mod utils; pub use utils::strings::{ c_char_to_string, - c_char_from_rc, kw_from_string, string_to_c_char, }; @@ -276,7 +275,7 @@ pub unsafe extern "C" fn typed_value_as_entid(typed_value: *mut TypedValue) -> #[no_mangle] pub unsafe extern "C" fn typed_value_as_kw(typed_value: *mut TypedValue) -> *const c_char { let typed_value = Box::from_raw(typed_value); - string_to_c_char(typed_value.into_kw().expect("Typed value cannot be coerced into a Namespaced Keyword").to_string()) + typed_value.into_kw_c_string().expect("Typed value cannot be coerced into a Namespaced Keyword") } //as_boolean @@ -305,20 +304,20 @@ pub unsafe extern "C" fn typed_value_as_timestamp(typed_value: *mut TypedValue) #[no_mangle] pub unsafe extern "C" fn typed_value_as_string(typed_value: *mut TypedValue) -> *const c_char { let typed_value = Box::from_raw(typed_value); - c_char_from_rc(typed_value.into_string().expect("Typed value cannot be coerced into a String")) + typed_value.into_c_string().expect("Typed value cannot be coerced into a String") } //as_uuid #[no_mangle] pub unsafe extern "C" fn typed_value_as_uuid(typed_value: *mut TypedValue) -> *const c_char { let typed_value = Box::from_raw(typed_value); - string_to_c_char(typed_value.into_uuid_string().expect("Typed value cannot be coerced into a Uuid")) + typed_value.into_uuid_c_string().expect("Typed value cannot be coerced into a String") } #[no_mangle] pub unsafe extern "C" fn row_at_index(rows: *mut Vec>, index: c_int) -> *mut Vec { let result = &*rows; - result.get(index as usize).map_or(std::ptr::null_mut(), |v| Box::into_raw(Box::new(v.clone()))) + result.get(index as usize).map_or_else(std::ptr::null_mut, |v| Box::into_raw(Box::new(v.clone()))) } #[no_mangle] @@ -342,62 +341,62 @@ pub unsafe extern "C" fn values_iter(values: *mut Vec) -> *mut Type #[no_mangle] pub unsafe extern "C" fn values_iter_next(iter: *mut TypedValueIterator) -> *const TypedValue { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| &v as *const TypedValue) + iter.next().map_or_else(std::ptr::null, |v| &v as *const TypedValue) } //as_long #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_long(iter: *mut TypedValueIterator) -> *const i64 { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| &v.into_long().expect("Typed value cannot be coerced into a Long") as *const i64) + iter.next().map_or_else(std::ptr::null, |v| &v.into_long().expect("Typed value cannot be coerced into a Long") as *const i64) } // as ref #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_entid(iter: *mut TypedValueIterator) -> *const Entid { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| &v.into_entid().expect("Typed value cannot be coerced into am Entid") as *const Entid) + iter.next().map_or_else(std::ptr::null, |v| &v.into_entid().expect("Typed value cannot be coerced into am Entid") as *const Entid) } // as kw #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_kw(iter: *mut TypedValueIterator) -> *const c_char { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| string_to_c_char(v.into_kw().expect("Typed value cannot be coerced into a Namespaced Keyword").to_string())) + iter.next().map_or_else(std::ptr::null, |v| v.into_kw_c_string().expect("Typed value cannot be coerced into a Namespaced Keyword")) } //as_boolean #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_boolean(iter: *mut TypedValueIterator) -> *const bool { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| &v.into_boolean().expect("Typed value cannot be coerced into a Boolean") as *const bool) + iter.next().map_or_else(std::ptr::null, |v| &v.into_boolean().expect("Typed value cannot be coerced into a Boolean") as *const bool) } //as_double #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_double(iter: *mut TypedValueIterator) -> *const f64 { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| &v.into_double().expect("Typed value cannot be coerced into a Double") as *const f64) + iter.next().map_or_else(std::ptr::null, |v| &v.into_double().expect("Typed value cannot be coerced into a Double") as *const f64) } //as_timestamp #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_timestamp(iter: *mut TypedValueIterator) -> *const i64 { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| v.into_timestamp().expect("Typed value cannot be coerced into a Timestamp") as *const i64) + iter.next().map_or_else(std::ptr::null, |v| v.into_timestamp().expect("Typed value cannot be coerced into a Timestamp") as *const i64) } //as_string #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_string(iter: *mut TypedValueIterator) -> *const c_char { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| c_char_from_rc(v.into_string().expect("Typed value cannot be coerced into a String"))) + iter.next().map_or_else(std::ptr::null, |v| v.into_c_string().expect("Typed value cannot be coerced into a String")) } //as_uuid #[no_mangle] pub unsafe extern "C" fn values_iter_next_as_uuid(iter: *mut TypedValueIterator) -> *const c_char { let iter = &mut *iter; - iter.next().map_or(std::ptr::null_mut(), |v| string_to_c_char(v.into_uuid_string().expect("Typed value cannot be coerced into a Uuid"))) + iter.next().map_or_else(std::ptr::null, |v| v.into_uuid_c_string().expect("Typed value cannot be coerced into a Uuid")) } #[no_mangle] @@ -426,7 +425,7 @@ pub unsafe extern "C" fn value_at_index_as_entid(values: *mut Vec, i pub unsafe extern "C" fn value_at_index_as_kw(values: *mut Vec, index: c_int) -> *const c_char { let result = &*values; let value = result.get(index as usize).expect("No value at index"); - string_to_c_char(value.clone().into_kw().expect("Typed value cannot be coerced into a Namespaced Keyword").to_string()) + value.clone().into_kw_c_string().expect("Typed value cannot be coerced into a Namespaced Keyword") } //as_boolean @@ -458,7 +457,7 @@ pub unsafe extern "C" fn value_at_index_as_timestamp(values: *mut Vec, index: c_int) -> *mut c_char { let result = &*values; let value = result.get(index as usize).expect("No value at index"); - c_char_from_rc(value.clone().into_string().expect("Typed value cannot be coerced into a String")) + value.clone().into_c_string().expect("Typed value cannot be coerced into a String") } //as_uuid @@ -466,7 +465,7 @@ pub unsafe extern "C" fn value_at_index_as_string(values: *mut Vec, pub unsafe extern "C" fn value_at_index_as_uuid(values: *mut Vec, index: c_int) -> *mut c_char { let result = &*values; let value = result.get(index as usize).expect("No value at index"); - string_to_c_char(value.clone().into_uuid_string().expect("Typed value cannot be coerced into a Uuid")) + value.clone().into_uuid_c_string().expect("Typed value cannot be coerced into a Uuid") } // TODO: q_prepare diff --git a/ffi/src/utils.rs b/ffi/src/utils.rs index 97a6111d..42e812ce 100644 --- a/ffi/src/utils.rs +++ b/ffi/src/utils.rs @@ -9,13 +9,11 @@ // specific language governing permissions and limitations under the License. pub mod strings { - use std; use std::ffi::{ CString, CStr }; use std::os::raw::c_char; - use std::rc::Rc; use mentat::{ NamespacedKeyword, @@ -31,19 +29,12 @@ pub mod strings { CString::new(r_string.into()).unwrap().into_raw() } + // TODO: validate. The input might not be a keyword! pub fn kw_from_string(mut keyword_string: String) -> NamespacedKeyword { let attr_name = keyword_string.split_off(1); let parts: Vec<&str> = attr_name.split("/").collect(); NamespacedKeyword::new(parts[0], parts[1]) } - - pub fn c_char_from_rc(rc_string: Rc) -> *mut c_char { - if let Some(str_ptr) = unsafe { Rc::into_raw(rc_string).as_ref() } { - string_to_c_char(str_ptr.clone()) - } else { - std::ptr::null_mut() - } - } } pub mod log { diff --git a/query-algebrizer/src/clauses/convert.rs b/query-algebrizer/src/clauses/convert.rs index fd72a36e..9869f2c2 100644 --- a/query-algebrizer/src/clauses/convert.rs +++ b/query-algebrizer/src/clauses/convert.rs @@ -8,8 +8,6 @@ // 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::{ HasSchema, Schema, @@ -166,7 +164,7 @@ impl ConjoiningClauses { (true, true) => { // Ambiguous: this could be a keyword or an ident. // Default to keyword. - Ok(Val(TypedValue::Keyword(Rc::new(x)))) + Ok(Val(x.into())) }, (true, false) => { // This can only be an ident. Look it up! @@ -176,7 +174,7 @@ impl ConjoiningClauses { } }, (false, true) => { - Ok(Val(TypedValue::Keyword(Rc::new(x)))) + Ok(Val(TypedValue::Keyword(x.into()))) }, (false, false) => { Ok(Impossible(EmptyBecause::TypeMismatch { diff --git a/query-algebrizer/src/clauses/fulltext.rs b/query-algebrizer/src/clauses/fulltext.rs index f01054cc..53121bc3 100644 --- a/query-algebrizer/src/clauses/fulltext.rs +++ b/query-algebrizer/src/clauses/fulltext.rs @@ -260,8 +260,6 @@ impl ConjoiningClauses { mod testing { use super::*; - use std::rc::Rc; - use mentat_core::{ Attribute, Schema, @@ -309,7 +307,7 @@ mod testing { args: vec![ FnArg::SrcVar(SrcVar::DefaultSrc), FnArg::IdentOrKeyword(NamespacedKeyword::new("foo", "fts")), - FnArg::Constant(NonIntegerConstant::Text(Rc::new("needle".into()))), + FnArg::Constant("needle".into()), ], binding: Binding::BindRel(vec![VariableOrPlaceholder::Variable(Variable::from_valid_name("?entity")), VariableOrPlaceholder::Variable(Variable::from_valid_name("?value")), @@ -331,7 +329,7 @@ mod testing { assert_eq!(clauses.0[1], ColumnConstraint::Equals(QualifiedAlias("datoms01".to_string(), Column::Fixed(DatomsColumn::Value)), QueryValue::Column(QualifiedAlias("fulltext_values00".to_string(), Column::Fulltext(FulltextColumn::Rowid)))).into()); assert_eq!(clauses.0[2], ColumnConstraint::Matches(QualifiedAlias("fulltext_values00".to_string(), Column::Fulltext(FulltextColumn::Text)), - QueryValue::TypedValue(TypedValue::String(Rc::new("needle".into())))).into()); + QueryValue::TypedValue("needle".into())).into()); let bindings = cc.column_bindings; assert_eq!(bindings.len(), 3); @@ -367,7 +365,7 @@ mod testing { args: vec![ FnArg::SrcVar(SrcVar::DefaultSrc), FnArg::IdentOrKeyword(NamespacedKeyword::new("foo", "bar")), - FnArg::Constant(NonIntegerConstant::Text(Rc::new("needle".into()))), + FnArg::Constant("needle".into()), ], binding: Binding::BindRel(vec![VariableOrPlaceholder::Variable(Variable::from_valid_name("?entity")), VariableOrPlaceholder::Variable(Variable::from_valid_name("?value")), diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index a361de02..7ee8b136 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -27,6 +27,7 @@ use std::fmt::{ use mentat_core::{ Attribute, + Cloned, Entid, HasSchema, KnownEntid, @@ -96,17 +97,6 @@ pub use self::inputs::QueryInputs; use Known; -// 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() - } -} - trait Contains { fn when_contains T>(&self, k: &K, f: F) -> Option; } @@ -1159,7 +1149,7 @@ fn add_attribute(schema: &mut Schema, e: Entid, a: Attribute) { #[cfg(test)] pub(crate) fn ident(ns: &str, name: &str) -> PatternNonValuePlace { - PatternNonValuePlace::Ident(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) + NamespacedKeyword::new(ns, name).into() } #[cfg(test)] diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index c5aa52e0..6ab4e8e6 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -88,7 +88,6 @@ impl ConjoiningClauses { mod testing { extern crate mentat_query_parser; - use std::rc::Rc; use std::collections::BTreeSet; use super::*; @@ -469,7 +468,9 @@ mod testing { :where [?x :foo/knows "Bill"] (not [?x :foo/knows ?y])]"#; - let inputs = QueryInputs::with_value_sequence(vec![(Variable::from_valid_name("?y"),TypedValue::String(Rc::new("John".to_string())))]); + let inputs = QueryInputs::with_value_sequence(vec![ + (Variable::from_valid_name("?y"), "John".into()) + ]); let cc = alg_with_inputs(&schema, query, inputs); let vx = Variable::from_valid_name("?x"); diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index 28ea8a79..05b08c24 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -9,6 +9,7 @@ // specific language governing permissions and limitations under the License. use mentat_core::{ + Cloned, Entid, HasSchema, TypedValue, @@ -24,8 +25,6 @@ use mentat_query::{ Variable, }; -use super::RcCloned; - use clauses::{ ConjoiningClauses, }; @@ -646,7 +645,6 @@ mod testing { use std::collections::BTreeMap; use std::collections::BTreeSet; - use std::rc::Rc; use mentat_core::attribute::Unique; use mentat_core::{ @@ -827,7 +825,7 @@ mod testing { let v = Variable::from_valid_name("?v"); cc.input_variables.insert(a.clone()); - cc.value_bindings.insert(a.clone(), TypedValue::Keyword(Rc::new(NamespacedKeyword::new("foo", "bar")))); + cc.value_bindings.insert(a.clone(), TypedValue::typed_ns_keyword("foo", "bar")); let known = Known::for_schema(&schema); cc.apply_parsed_pattern(known, Pattern { source: None, @@ -931,7 +929,7 @@ mod testing { source: None, entity: PatternNonValuePlace::Variable(x.clone()), attribute: PatternNonValuePlace::Placeholder, - value: PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new("hello".to_string()))), + value: PatternValuePlace::Constant("hello".into()), tx: PatternNonValuePlace::Placeholder, }); @@ -954,7 +952,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(Rc::new("hello".to_string())))), + ColumnConstraint::Equals(d0_v, QueryValue::TypedValue(TypedValue::typed_string("hello"))), ColumnConstraint::has_unit_type("all_datoms00".to_string(), ValueType::String), ].into()); } @@ -982,7 +980,7 @@ mod testing { source: None, entity: PatternNonValuePlace::Variable(x.clone()), attribute: ident("foo", "roz"), - value: PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new("idgoeshere".to_string()))), + value: PatternValuePlace::Constant("idgoeshere".into()), tx: PatternNonValuePlace::Placeholder, }); cc.apply_parsed_pattern(known, Pattern { diff --git a/query-algebrizer/src/types.rs b/query-algebrizer/src/types.rs index 1684cb3a..b8b75322 100644 --- a/query-algebrizer/src/types.rs +++ b/query-algebrizer/src/types.rs @@ -15,11 +15,10 @@ use std::fmt::{ Result, }; -use std::rc::Rc; - use mentat_core::{ Entid, TypedValue, + ValueRc, ValueType, ValueTypeSet, }; @@ -709,7 +708,7 @@ pub enum EvolvedValuePlace { Entid(Entid), Value(TypedValue), EntidOrInteger(i64), - IdentOrKeyword(Rc), + IdentOrKeyword(ValueRc), } pub enum PlaceOrEmpty { diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index 25a23f9e..56c0c11a 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -120,7 +120,7 @@ mod tests { }; fn value_ident(ns: &str, name: &str) -> PatternValuePlace { - PatternValuePlace::IdentOrKeyword(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) + NamespacedKeyword::new(ns, name).into() } /// Tests that the top-level form is a valid `or`, returning the clauses. diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs index 0d8e17f0..3d982a3f 100644 --- a/query-parser/src/parse.rs +++ b/query-parser/src/parse.rs @@ -626,7 +626,7 @@ mod test { } fn ident_kw(kw: edn::NamespacedKeyword) -> PatternNonValuePlace { - PatternNonValuePlace::Ident(Rc::new(kw)) + PatternNonValuePlace::Ident(kw.into()) } fn ident(ns: &str, name: &str) -> PatternNonValuePlace { diff --git a/query-parser/tests/find_tests.rs b/query-parser/tests/find_tests.rs index d0873bce..a911177c 100644 --- a/query-parser/tests/find_tests.rs +++ b/query-parser/tests/find_tests.rs @@ -16,8 +16,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_parser; -use std::rc::Rc; - use edn::{ NamespacedKeyword, PlainSymbol, @@ -164,7 +162,7 @@ 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))) + NamespacedKeyword::new(ns, name).into() } #[test] @@ -283,7 +281,7 @@ fn can_parse_uuid() { WhereClause::Pattern( Pattern::new(None, PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), - PatternNonValuePlace::Ident(Rc::new(NamespacedKeyword::new("foo", "baz"))), + NamespacedKeyword::new("foo", "baz").into(), PatternValuePlace::Constant(NonIntegerConstant::Uuid(expected)), PatternNonValuePlace::Placeholder) .expect("valid pattern"))); diff --git a/query-sql/src/lib.rs b/query-sql/src/lib.rs index deeca799..1f72144f 100644 --- a/query-sql/src/lib.rs +++ b/query-sql/src/lib.rs @@ -761,7 +761,7 @@ mod tests { let c = Constraint::Infix { op: Op("MATCHES"), left: ColumnOrExpression::Column(QualifiedAlias("fulltext01".to_string(), Column::Fulltext(FulltextColumn::Text))), - right: ColumnOrExpression::Value(TypedValue::String(Rc::new("needle".to_string()))), + right: ColumnOrExpression::Value("needle".into()), }; let q = build_query(&c); assert_eq!("`fulltext01`.text MATCHES $v0", q.sql); diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index e0530745..bc3676f6 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -739,7 +739,7 @@ fn test_ground_scalar() { // Verify that we accept bound input constants. let query = r#"[:find ?x . :in ?v :where [(ground ?v) ?x]]"#; - let inputs = QueryInputs::with_value_sequence(vec![(Variable::from_valid_name("?v"), TypedValue::String(Rc::new("aaa".into())))]); + let inputs = QueryInputs::with_value_sequence(vec![(Variable::from_valid_name("?v"), "aaa".into())]); let constant = translate_with_inputs_to_constant(&schema, query, inputs); assert_eq!(constant.project_without_rows().unwrap() .into_scalar().unwrap(), @@ -760,7 +760,7 @@ fn test_ground_tuple() { // Verify that we accept bound input constants. let query = r#"[:find [?x ?y] :in ?u ?v :where [(ground [?u ?v]) [?x ?y]]]"#; let inputs = QueryInputs::with_value_sequence(vec![(Variable::from_valid_name("?u"), TypedValue::Long(2)), - (Variable::from_valid_name("?v"), TypedValue::String(Rc::new("aaa".into()))),]); + (Variable::from_valid_name("?v"), "aaa".into()),]); let constant = translate_with_inputs_to_constant(&schema, query, inputs); assert_eq!(constant.project_without_rows().unwrap() diff --git a/query/src/lib.rs b/query/src/lib.rs index 1eea007e..f8c4cdcb 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -55,7 +55,9 @@ pub use edn::{ }; use mentat_core::{ + FromRc, TypedValue, + ValueRc, ValueType, }; @@ -205,7 +207,7 @@ pub enum NonIntegerConstant { Boolean(bool), BigInteger(BigInt), Float(OrderedFloat), - Text(Rc), + Text(ValueRc), Instant(DateTime), Uuid(Uuid), } @@ -216,13 +218,25 @@ impl NonIntegerConstant { NonIntegerConstant::BigInteger(_) => unimplemented!(), // TODO: #280. NonIntegerConstant::Boolean(v) => TypedValue::Boolean(v), NonIntegerConstant::Float(v) => TypedValue::Double(v), - NonIntegerConstant::Text(v) => TypedValue::String(v), + NonIntegerConstant::Text(v) => v.into(), NonIntegerConstant::Instant(v) => TypedValue::Instant(v), NonIntegerConstant::Uuid(v) => TypedValue::Uuid(v), } } } +impl<'a> From<&'a str> for NonIntegerConstant { + fn from(val: &'a str) -> NonIntegerConstant { + NonIntegerConstant::Text(ValueRc::new(val.to_string())) + } +} + +impl From for NonIntegerConstant { + fn from(val: String) -> NonIntegerConstant { + NonIntegerConstant::Text(ValueRc::new(val)) + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub enum FnArg { Variable(Variable), @@ -260,7 +274,7 @@ impl FromValue for FnArg { Some(FnArg::Constant(NonIntegerConstant::BigInteger(x.clone()))), Text(ref x) => // TODO: intern strings. #398. - Some(FnArg::Constant(NonIntegerConstant::Text(Rc::new(x.clone())))), + Some(FnArg::Constant(x.clone().into())), Nil | NamespacedSymbol(_) | Keyword(_) | @@ -312,7 +326,19 @@ pub enum PatternNonValuePlace { Placeholder, Variable(Variable), Entid(i64), // Will always be +ve. See #190. - Ident(Rc), + Ident(ValueRc), +} + +impl From> for PatternNonValuePlace { + fn from(value: Rc) -> Self { + PatternNonValuePlace::Ident(ValueRc::from_rc(value)) + } +} + +impl From for PatternNonValuePlace { + fn from(value: NamespacedKeyword) -> Self { + PatternNonValuePlace::Ident(ValueRc::new(value)) + } } impl PatternNonValuePlace { @@ -355,7 +381,7 @@ impl FromValue for PatternNonValuePlace { } }, edn::SpannedValue::NamespacedKeyword(ref x) => - Some(PatternNonValuePlace::Ident(Rc::new(x.clone()))), + Some(x.clone().into()), _ => None, } } @@ -363,7 +389,7 @@ impl FromValue for PatternNonValuePlace { #[derive(Clone, Debug, Eq, PartialEq)] pub enum IdentOrEntid { - Ident(Rc), + Ident(NamespacedKeyword), Entid(i64), } @@ -375,10 +401,22 @@ pub enum PatternValuePlace { Placeholder, Variable(Variable), EntidOrInteger(i64), - IdentOrKeyword(Rc), + IdentOrKeyword(ValueRc), Constant(NonIntegerConstant), } +impl From> for PatternValuePlace { + fn from(value: Rc) -> Self { + PatternValuePlace::IdentOrKeyword(ValueRc::from_rc(value)) + } +} + +impl From for PatternValuePlace { + fn from(value: NamespacedKeyword) -> Self { + PatternValuePlace::IdentOrKeyword(ValueRc::new(value)) + } +} + impl FromValue for PatternValuePlace { fn from_value(v: &edn::ValueAndSpan) -> Option { match v.inner { @@ -389,7 +427,7 @@ impl FromValue for PatternValuePlace { edn::SpannedValue::PlainSymbol(ref x) => Variable::from_symbol(x).map(PatternValuePlace::Variable), edn::SpannedValue::NamespacedKeyword(ref x) => - Some(PatternValuePlace::IdentOrKeyword(Rc::new(x.clone()))), + Some(x.clone().into()), edn::SpannedValue::Boolean(x) => Some(PatternValuePlace::Constant(NonIntegerConstant::Boolean(x))), edn::SpannedValue::Float(x) => @@ -400,7 +438,7 @@ impl FromValue for PatternValuePlace { Some(PatternValuePlace::Constant(NonIntegerConstant::Instant(x))), edn::SpannedValue::Text(ref x) => // TODO: intern strings. #398. - Some(PatternValuePlace::Constant(NonIntegerConstant::Text(Rc::new(x.clone())))), + Some(PatternValuePlace::Constant(x.clone().into())), edn::SpannedValue::Uuid(ref u) => Some(PatternValuePlace::Constant(NonIntegerConstant::Uuid(u.clone()))), @@ -754,7 +792,7 @@ impl Pattern { return Some(Pattern { source: src, entity: v_e, - attribute: PatternNonValuePlace::Ident(Rc::new(k.to_reversed())), + attribute: k.to_reversed().into(), value: e_v, tx: tx, }); diff --git a/sql/src/lib.rs b/sql/src/lib.rs index 84583d17..6e2e845a 100644 --- a/sql/src/lib.rs +++ b/sql/src/lib.rs @@ -24,6 +24,7 @@ use ordered_float::OrderedFloat; use mentat_core::{ ToMicros, TypedValue, + ValueRc, }; pub use rusqlite::types::Value; @@ -103,7 +104,7 @@ pub struct SQLiteQueryBuilder { // Instead we track byte and String arguments separately, mapping them to their argument name, // in order to dedupe. We'll add these to the regular argument vector later. byte_args: HashMap, String>, // From value to argument name. - string_args: HashMap, String>, // From value to argument name. + string_args: HashMap, String>, // From value to argument name. args: Vec<(String, Rc)>, // (arg, value). } diff --git a/tests/query.rs b/tests/query.rs index 2d10d557..1ad0b02d 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -1321,22 +1321,22 @@ fn test_aggregation_implicit_grouping() { assert_eq!(vals, vec![ vec![TypedValue::Ref(ids.get("a").cloned().unwrap()), - TypedValue::String("Alice".to_string().into()), + "Alice".into(), TypedValue::Long(99), TypedValue::Long(3), TypedValue::Double((127f64 / 3f64).into())], vec![TypedValue::Ref(ids.get("b").cloned().unwrap()), - TypedValue::String("Beli".to_string().into()), + "Beli".into(), TypedValue::Long(22), TypedValue::Long(2), TypedValue::Double((33f64 / 2f64).into())], vec![TypedValue::Ref(ids.get("c").cloned().unwrap()), - TypedValue::String("Carlos".to_string().into()), + "Carlos".into(), TypedValue::Long(42), TypedValue::Long(1), TypedValue::Double(42f64.into())], vec![TypedValue::Ref(ids.get("d").cloned().unwrap()), - TypedValue::String("Diana".to_string().into()), + "Diana".into(), TypedValue::Long(28), TypedValue::Long(2), TypedValue::Double((33f64 / 2f64).into())]].into()); @@ -1454,6 +1454,6 @@ fn test_tx_data() { } }; - assert_tx_data(&store, &tx1, TypedValue::String("1".to_string().into())); - assert_tx_data(&store, &tx2, TypedValue::String("2".to_string().into())); + assert_tx_data(&store, &tx1, "1".into()); + assert_tx_data(&store, &tx2, "2".into()); }