From 1918388ddffefd7f44df7e0bcd7aa8f0ded8a452 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Wed, 25 Apr 2018 12:22:15 -0700 Subject: [PATCH] Introduce ValueRc as an abstraction over Rc/Arc choice. --- core/src/lib.rs | 2 + core/src/types.rs | 49 +++++++++++++++++-------- db/src/cache.rs | 5 +-- db/src/db.rs | 5 ++- query-algebrizer/src/clauses/mod.rs | 10 +++-- query-algebrizer/src/clauses/pattern.rs | 2 +- query-algebrizer/src/types.rs | 5 +-- query/src/lib.rs | 22 ++++++----- sql/src/lib.rs | 3 +- 9 files changed, 64 insertions(+), 39 deletions(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index a2095964..04891f19 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -56,11 +56,13 @@ mod sql_types; pub use types::{ Entid, + FromRc, KnownEntid, TypedValue, Binding, ValueType, ValueTypeTag, + ValueRc, now, }; diff --git a/core/src/types.rs b/core/src/types.rs index fee9cda1..c7ea58df 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -72,6 +72,11 @@ impl FromRc for Arc where T: Sized + Clone { } } +// +// Use Rc for values. +// +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 @@ -208,8 +213,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. } @@ -228,8 +233,8 @@ 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 where T: Into { @@ -258,7 +263,7 @@ 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>); +pub struct StructuredMap(IndexMap, Binding>); impl Binding { /// Returns true if the provided type is `Some` and matches this value's type, or if the @@ -311,14 +316,14 @@ 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 { 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 { s.into() @@ -381,31 +386,43 @@ 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(value.clone()) + TypedValue::String(ValueRc::from_rc(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(value.clone()) + 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)) } } @@ -442,7 +459,7 @@ impl TypedValue { } } - pub fn into_kw(self) -> Option> { + pub fn into_kw(self) -> Option> { match self { TypedValue::Keyword(v) => Some(v), _ => None, @@ -484,7 +501,7 @@ impl TypedValue { } } - pub fn into_string(self) -> Option> { + pub fn into_string(self) -> Option> { match self { TypedValue::String(v) => Some(v), _ => None, @@ -521,7 +538,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, @@ -563,7 +580,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 564d5b44..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::{ @@ -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/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index 79f4d9ba..1903a62b 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -25,6 +25,10 @@ use std::fmt::{ Formatter, }; +use std::ops::{ + Deref, +}; + use mentat_core::{ Attribute, Entid, @@ -97,13 +101,13 @@ pub use self::inputs::QueryInputs; use Known; // We do this a lot for errors. -trait RcCloned { +trait Cloned { fn cloned(&self) -> T; } -impl RcCloned for ::std::rc::Rc { +impl Cloned for ::mentat_core::ValueRc { fn cloned(&self) -> T { - self.as_ref().clone() + self.deref().clone() } } diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index d92b3253..62aede94 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -24,7 +24,7 @@ use mentat_query::{ Variable, }; -use super::RcCloned; +use super::Cloned; use clauses::{ ConjoiningClauses, 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/src/lib.rs b/query/src/lib.rs index bd821065..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), } @@ -225,13 +227,13 @@ impl NonIntegerConstant { impl<'a> From<&'a str> for NonIntegerConstant { fn from(val: &'a str) -> NonIntegerConstant { - NonIntegerConstant::Text(Rc::new(val.to_string())) + NonIntegerConstant::Text(ValueRc::new(val.to_string())) } } impl From for NonIntegerConstant { fn from(val: String) -> NonIntegerConstant { - NonIntegerConstant::Text(Rc::new(val)) + NonIntegerConstant::Text(ValueRc::new(val)) } } @@ -324,18 +326,18 @@ 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(value.clone()) + PatternNonValuePlace::Ident(ValueRc::from_rc(value)) } } impl From for PatternNonValuePlace { fn from(value: NamespacedKeyword) -> Self { - PatternNonValuePlace::Ident(Rc::new(value)) + PatternNonValuePlace::Ident(ValueRc::new(value)) } } @@ -387,7 +389,7 @@ impl FromValue for PatternNonValuePlace { #[derive(Clone, Debug, Eq, PartialEq)] pub enum IdentOrEntid { - Ident(Rc), + Ident(NamespacedKeyword), Entid(i64), } @@ -399,19 +401,19 @@ 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(value.clone()) + PatternValuePlace::IdentOrKeyword(ValueRc::from_rc(value)) } } impl From for PatternValuePlace { fn from(value: NamespacedKeyword) -> Self { - PatternValuePlace::IdentOrKeyword(Rc::new(value)) + PatternValuePlace::IdentOrKeyword(ValueRc::new(value)) } } 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). }