diff --git a/core/src/lib.rs b/core/src/lib.rs index f66eac5a..59e02067 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -281,25 +281,33 @@ impl From for TypedValue { } } +/// Type safe representation of the possible return values from SQLite's `typeof` +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] +pub enum SQLTypeAffinity { + Null, // "null" + Integer, // "integer" + Real, // "real" + Text, // "text" + Blob, // "blob" +} + // Put this here rather than in `db` simply because it's widely needed. pub trait SQLValueType { fn value_type_tag(&self) -> i32; fn accommodates_integer(&self, int: i64) -> bool; + + /// Return a pair of the ValueTypeTag for this value type, and the SQLTypeAffinity required + /// to distinguish it from any other types that share the same tag. + /// + /// Background: The tag alone is not enough to determine the type of a value, since multiple + /// ValueTypes may share the same tag (for example, ValueType::Long and ValueType::Double). + /// However, each ValueType can be determined by checking both the tag and the type's affinity. + fn sql_representation(&self) -> (ValueTypeTag, Option); } impl SQLValueType for ValueType { fn value_type_tag(&self) -> i32 { - match *self { - ValueType::Ref => 0, - ValueType::Boolean => 1, - ValueType::Instant => 4, - // SQLite distinguishes integral from decimal types, allowing long and double to share a tag. - ValueType::Long => 5, - ValueType::Double => 5, - ValueType::String => 10, - ValueType::Uuid => 11, - ValueType::Keyword => 13, - } + self.sql_representation().0 } /// Returns true if the provided integer is in the SQLite value space of this type. For @@ -326,6 +334,20 @@ impl SQLValueType for ValueType { Uuid => false, } } + + fn sql_representation(&self) -> (ValueTypeTag, Option) { + match *self { + ValueType::Ref => (0, None), + ValueType::Boolean => (1, None), + ValueType::Instant => (4, None), + // SQLite distinguishes integral from decimal types, allowing long and double to share a tag. + ValueType::Long => (5, Some(SQLTypeAffinity::Integer)), + ValueType::Double => (5, Some(SQLTypeAffinity::Real)), + ValueType::String => (10, None), + ValueType::Uuid => (11, None), + ValueType::Keyword => (13, None), + } + } } trait EnumSetExtensions { diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index a71c4c32..c65d09f0 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -934,7 +934,7 @@ impl ConjoiningClauses { self.wheres.add_intersection(ColumnConstraint::HasTypes { value: qa.0.clone(), value_types: *types, - strict: true, + check_value: true, }); } if let Some(reason) = empty_because { diff --git a/query-algebrizer/src/types.rs b/query-algebrizer/src/types.rs index e37765dd..e7dd2853 100644 --- a/query-algebrizer/src/types.rs +++ b/query-algebrizer/src/types.rs @@ -337,7 +337,7 @@ pub enum ColumnConstraint { HasTypes { value: TableAlias, value_types: ValueTypeSet, - strict: bool, + check_value: bool, }, NotExists(ComputedTable), Matches(QualifiedAlias, QueryValue), @@ -348,7 +348,7 @@ impl ColumnConstraint { ColumnConstraint::HasTypes { value, value_types: ValueTypeSet::of_one(value_type), - strict: false + check_value: false, } } } @@ -465,12 +465,12 @@ impl Debug for ColumnConstraint { write!(f, "{:?} MATCHES {:?}", qa, thing) }, - &HasTypes { ref value, ref value_types, strict } => { + &HasTypes { ref value, ref value_types, check_value } => { // This is cludgey, but it's debug code. write!(f, "(")?; for value_type in value_types.iter() { write!(f, "({:?}.value_type_tag = {:?}", value, value_type)?; - if strict && value_type == ValueType::Double || value_type == ValueType::Long { + if check_value && value_type == ValueType::Double || value_type == ValueType::Long { write!(f, " AND typeof({:?}) = '{:?}')", value, if value_type == ValueType::Double { "real" } else { "integer" })?; } else { diff --git a/query-sql/src/lib.rs b/query-sql/src/lib.rs index 8bcee6a6..3dfe413c 100644 --- a/query-sql/src/lib.rs +++ b/query-sql/src/lib.rs @@ -19,6 +19,7 @@ use std::boxed::Box; use mentat_core::{ Entid, TypedValue, + SQLTypeAffinity, }; use mentat_query::{ @@ -112,15 +113,6 @@ pub enum Constraint { } } -/// Type safe representation of the possible return values from `typeof` -pub enum SQLTypeAffinity { - Null, // "null" - Integer, // "integer" - Real, // "real" - Text, // "text" - Blob, // "blob" -} - impl Constraint { pub fn not_equal(left: ColumnOrExpression, right: ColumnOrExpression) -> Constraint { Constraint::Infix { @@ -326,19 +318,6 @@ impl QueryFragment for Op { } } -impl QueryFragment for SQLTypeAffinity { - fn push_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult { - out.push_sql(match *self { - SQLTypeAffinity::Null => "'null'", - SQLTypeAffinity::Integer => "'integer'", - SQLTypeAffinity::Real => "'real'", - SQLTypeAffinity::Text => "'text'", - SQLTypeAffinity::Blob => "'blob'", - }); - Ok(()) - } -} - impl QueryFragment for Constraint { fn push_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult { use self::Constraint::*; @@ -398,7 +377,13 @@ impl QueryFragment for Constraint { out.push_sql("typeof("); value.push_sql(out)?; out.push_sql(") = "); - affinity.push_sql(out)?; + out.push_sql(match *affinity { + SQLTypeAffinity::Null => "'null'", + SQLTypeAffinity::Integer => "'integer'", + SQLTypeAffinity::Real => "'real'", + SQLTypeAffinity::Text => "'text'", + SQLTypeAffinity::Blob => "'blob'", + }); Ok(()) }, } diff --git a/query-translator/src/translate.rs b/query-translator/src/translate.rs index a361c22f..ed5ff195 100644 --- a/query-translator/src/translate.rs +++ b/query-translator/src/translate.rs @@ -9,6 +9,7 @@ // specific language governing permissions and limitations under the License. use mentat_core::{ + SQLTypeAffinity, SQLValueType, TypedValue, ValueType, @@ -51,12 +52,13 @@ use mentat_query_sql::{ ProjectedColumn, Projection, SelectQuery, - SQLTypeAffinity, TableList, TableOrSubquery, Values, }; +use std::collections::HashMap; + use super::Result; trait ToConstraint { @@ -99,6 +101,51 @@ impl ToConstraint for ColumnConstraintOrAlternation { } } +fn affinity_count(tag: i32) -> usize { + ValueTypeSet::any().into_iter() + .filter(|t| t.value_type_tag() == tag) + .count() +} + +fn type_constraint(table: &TableAlias, tag: i32, to_check: Option>) -> Constraint { + let type_column = QualifiedAlias::new(table.clone(), + DatomsColumn::ValueTypeTag).to_column(); + let check_type_tag = Constraint::equal(type_column, ColumnOrExpression::Integer(tag)); + if let Some(affinities) = to_check { + let check_affinities = Constraint::Or { + constraints: affinities.into_iter().map(|affinity| { + Constraint::TypeCheck { + value: QualifiedAlias::new(table.clone(), + DatomsColumn::Value).to_column(), + affinity, + } + }).collect() + }; + Constraint::And { + constraints: vec![ + check_type_tag, + check_affinities + ] + } + } else { + check_type_tag + } +} + +// Returns a map of tags to a vector of all the possible affinities that those tags can represent +// given the types in `value_types`. +fn possible_affinities(value_types: ValueTypeSet) -> HashMap> { + let mut result = HashMap::new(); + for ty in value_types { + let (tag, affinity_to_check) = ty.sql_representation(); + let mut affinities = result.entry(tag).or_insert_with(Vec::new); + if let Some(affinity) = affinity_to_check { + affinities.push(affinity); + } + } + result +} + impl ToConstraint for ColumnConstraint { fn to_constraint(self) -> Constraint { use self::ColumnConstraint::*; @@ -159,52 +206,23 @@ impl ToConstraint for ColumnConstraint { right: right.into(), } }, - HasTypes { value: table, value_types, strict: strict_requested } => { - // If strict mode checking is on, and need to check exactly 1 - // (not both or neither) of ValueType::Double and ValueType::Long - // we emit a Constraint::TypeCheck. - let num_numeric_checks = (value_types.contains(ValueType::Double) as i32) + - (value_types.contains(ValueType::Long) as i32); - let strict = strict_requested && num_numeric_checks == 1; - - let types = if !strict && num_numeric_checks == 2 { - // If we aren't operating in strict mode (either because it - // wasn't requested, or because it doesn't make a difference), - // and both ValueType::Double and ValueType::Long are being - // checked, we remove the test for one of them, as they're - // represented using the same value type tag (we choose to - // remove the check for ValueType::Double, but it's an - // arbitrary choice) - value_types.difference(&ValueTypeSet::of_one(ValueType::Double)) + HasTypes { value: table, value_types, check_value } => { + let constraints = if check_value { + possible_affinities(value_types) + .into_iter() + .map(|(tag, affinities)| { + let to_check = if affinities.len() == 0 || affinities.len() == affinity_count(tag) { + None + } else { + Some(affinities) + }; + type_constraint(&table, tag, to_check) + }).collect() } else { - value_types + value_types.into_iter() + .map(|vt| type_constraint(&table, vt.value_type_tag(), None)) + .collect() }; - let constraints = types.into_iter().map(|ty| { - let type_column = QualifiedAlias::new(table.clone(), DatomsColumn::ValueTypeTag).to_column(); - let loose = Constraint::equal(type_column, ColumnOrExpression::Integer(ty.value_type_tag())); - if !strict || (ty != ValueType::Long && ty != ValueType::Double) { - loose - } else { - let val_column = QualifiedAlias::new(table.clone(), DatomsColumn::Value).to_column(); - // We're handling strict equality for a numeric type, so we need to emit - // a `typeof(col) = '(real|integer)'` too. This should happen a maximum of - // once per `HasTypes` - Constraint::And { - constraints: vec![ - loose, - Constraint::TypeCheck { - value: val_column, - affinity: match ty { - ValueType::Long => SQLTypeAffinity::Integer, - ValueType::Double => SQLTypeAffinity::Real, - _ => unreachable!() - } - } - ] - } - } - }).collect::>(); - Constraint::Or { constraints } }, diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index cc1ae2e9..76df7245 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -296,7 +296,7 @@ fn test_type_required_long() { assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` \ FROM `datoms` AS `datoms00` \ WHERE ((`datoms00`.value_type_tag = 5 AND \ - typeof(`datoms00`.v) = 'integer'))"); + (typeof(`datoms00`.v) = 'integer')))"); assert_eq!(args, vec![]); } @@ -311,7 +311,7 @@ fn test_type_required_double() { assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` \ FROM `datoms` AS `datoms00` \ WHERE ((`datoms00`.value_type_tag = 5 AND \ - typeof(`datoms00`.v) = 'real'))"); + (typeof(`datoms00`.v) = 'real')))"); assert_eq!(args, vec![]); }