From 13be228ab76fd5d892a90e80b19dde90ac71f056 Mon Sep 17 00:00:00 2001 From: Emily Toop Date: Mon, 19 Jun 2017 17:31:48 +0100 Subject: [PATCH] Remove everything except adding matching type tag constraints. * This is because all other ambigous vars are already held inside `extracted_types` and so we don't need a separate structure to remember them --- query-algebrizer/src/clauses/mod.rs | 221 ++-------------------------- query-algebrizer/src/clauses/not.rs | 1 - query-algebrizer/src/clauses/or.rs | 1 - query-translator/tests/translate.rs | 8 +- 4 files changed, 15 insertions(+), 216 deletions(-) diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index f124c5d2..501bb4ef 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -293,7 +293,7 @@ impl ConjoiningClauses { value_bindings: values, ..Default::default() }; - + // Pre-fill our type mappings with the types of the input bindings. cc.known_types .extend(types.iter() @@ -807,158 +807,24 @@ impl ConjoiningClauses { } - /// When a CC has accumulated all patterns, generate value_type_tag entries in `wheres` - /// to refine value types for which two things are true: - /// - /// - There are two or more different types with the same SQLite representation. E.g., - /// ValueType::Boolean shares a representation with Integer and Ref. - /// - There is no attribute constraint present in the CC. - /// - /// It's possible at this point for the space of acceptable type tags to not intersect: e.g., - /// for the query - /// - /// ```edn - /// [:find ?x :where - /// [?x ?y true] - /// [?z ?y ?x]] - /// ``` - /// - /// where `?y` must simultaneously be a ref-typed attribute and a boolean-typed attribute. This - /// function deduces that and calls `self.mark_known_empty`. #293. + /// For each bound var. discover which ones have pub fn expand_type_tags(&mut self) { + let mut vars_to_remove = Vec::new(); for (v, tag) in self.extracted_types.iter() { - let cols = self.column_bindings.get(v).unwrap().clone(); - for col in cols { - if col.0 != tag.0 { - let value = QueryValue::Column(QualifiedAlias(col.0.clone(), Column::Fixed(DatomsColumn::ValueTypeTag))); - let alias = tag.clone(); - self.wheres.0.push(ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(alias, value))); - } - } - } - - let mut computed_types: BTreeMap = BTreeMap::default(); - - for computed_table in self.computed_tables.iter() { - match computed_table { - &ComputedTable::NamedValues { ref names, ref values } => { - for (i, item) in names.iter().enumerate() { - computed_types.insert(item.clone(), values[i].clone()); - } - }, - _ => (), - } - } - - let mut empty_reason: Option = None; - let mut wheres_to_add = vec![]; - for (v, value_type_set) in self.known_types.iter() { - if self.column_bindings.contains_key(v) { - let cols = self.column_bindings.get(v).unwrap().clone(); + if let Some(cols) = self.column_bindings.get(&v) { for col in cols { - match col.1 { - Column::Fixed(DatomsColumn::Value) => { - // attempt to figure out exactly which type is used - // TODO: But only if the attribute is a variable or not present and therefore of unknown type - let mut value_type: Option = None; - for constraint in self.wheres.0.iter() { - match constraint { - &ColumnConstraintOrAlternation::Constraint(ref c) => { - match c { - &ColumnConstraint::Equals(ref alias, ref value) if alias.0 == col.0 && alias.1 == col.1 => { - match value { - &QueryValue::TypedValue(ref typed) => { - if value_type_set.contains(typed.value_type()) { - value_type = Some(typed.value_type().clone()); - } else { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(typed.value_type())}); - } - }, - &QueryValue::PrimitiveLong(_) => { - let desired = ValueType::Long; - if value_type_set.contains(desired) { - value_type = Some(desired); - } else { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(desired)}); - } - }, - &QueryValue::Column(ref alias) => { - let desired = if computed_types.contains_key(&v) { computed_types.get(&v).unwrap().value_type() } else { ValueType::Ref }; - if value_type_set.contains(desired) { - value_type = Some(desired); - } else { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(desired)}); - } - }, - &QueryValue::Entid(_) => (), - } - }, - &ColumnConstraint::NumericInequality { operator, ref left, ref right } => { - match left { - &QueryValue::Column(ref alias) if alias.0 == col.0 && alias.1 == col.1 => { - match right { - &QueryValue::TypedValue(ref typed) if value_type_set.contains(typed.value_type()) => { - value_type = Some(typed.value_type().clone()); - }, - &QueryValue::PrimitiveLong(_) if value_type_set.contains(ValueType::Long) => { - value_type = Some(ValueType::Long); - }, - _ => (), - } - }, - _ => (), - } - }, - &ColumnConstraint::HasType(ref alias, ref value_type) => { - if *alias == col.0 { - if !value_type_set.contains(value_type.clone()) { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(value_type.clone())}); - } - } - }, - _ => (), - } - }, - &ColumnConstraintOrAlternation::Alternation(_) => (), - } - } - - if value_type.is_some() { - let new_constraint = ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(col.0, value_type.expect("Expected type").clone())); - if !self.wheres.0.contains(&new_constraint) { - wheres_to_add.push(new_constraint); - } - } - }, - Column::Fixed(DatomsColumn::Entity) => (), - Column::Fixed(DatomsColumn::Attribute) => - for constraint in self.wheres.0.iter() { - match constraint { - &ColumnConstraintOrAlternation::Constraint(ref c) => { - match c { - &ColumnConstraint::HasType(ref alias, ref value_type) => { - if *alias == col.0 { - if !value_type_set.contains(value_type.clone()) { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(value_type.clone())}); - } - } - }, - _ => () - } - }, - &ColumnConstraintOrAlternation::Alternation(_) => (), - } - }, - _ => () + if col.0 != tag.0 { + let value = QueryValue::Column(QualifiedAlias(col.0.clone(), Column::Fixed(DatomsColumn::ValueTypeTag))); + self.wheres.0.push(ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(tag.clone(), value))); + vars_to_remove.push(v.clone()); } } } } - if empty_reason.is_some() { - self.mark_known_empty(empty_reason.unwrap()); - return; + + for v in vars_to_remove { + self.extracted_types.remove(&v); } - self.wheres.0.append(&mut wheres_to_add); } } @@ -1026,7 +892,6 @@ mod tests { use super::*; use self::mentat_query_parser::parse_find_string; - use types::NumericComparison; use algebrize; fn prepopulated_schema() -> Schema { @@ -1122,68 +987,4 @@ mod tests { ])); } - #[test] - fn test_expand_type_tags_mismatched_variables() { - let schema = prepopulated_schema(); - let query = r#" - [:find ?x - :where [?x ?y true] - [?z ?y ?x]]"#; - let parsed = parse_find_string(query).expect("parse failed"); - let cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - let vy = Variable::from_valid_name("?y"); - println!("{:?}", cc); - assert!(cc.is_known_empty()); - assert_eq!(cc.empty_because.unwrap(), EmptyBecause::TypeMismatch{var: vy, existing: ValueTypeSet::of_one(ValueType::Ref), desired: ValueTypeSet::of_one(ValueType::Boolean)}); - } - - #[test] - fn test_expand_type_tags_unambiguous_var_type() { - let schema = prepopulated_schema(); - let query = r#" - [:find ?x ?y ?z - :where - [?x _ ?y] - [?y _ ?z]]"#; - let parsed = parse_find_string(query).expect("parse failed"); - let cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - println!("{:?}", cc); - - let d0 = "all_datoms00".to_string(); - let d0v = QualifiedAlias::new(d0.clone(), DatomsColumn::Value); - - let d1 = "all_datoms01".to_string(); - let d1e = QualifiedAlias::new(d1.clone(), DatomsColumn::Entity); - - assert!(!cc.is_known_empty()); - assert_eq!(cc.wheres, ColumnIntersection(vec![ - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(d0v.clone(), QueryValue::Column(d1e.clone()))), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(d0, ValueType::Ref)), - ])); - } - - #[test] - fn test_expand_type_tags_infered_type() { - let schema = prepopulated_schema(); - let query = r#" - [:find ?x ?v - :where - [?x _ ?v] - [(< ?v 10)]]"#; - let parsed = parse_find_string(query).expect("parse failed"); - let cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - - let d0 = "all_datoms00".to_string(); - let d0v = QualifiedAlias::new(d0.clone(), DatomsColumn::Value); - - assert!(!cc.is_known_empty()); - assert_eq!(cc.wheres, ColumnIntersection(vec![ - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::NumericInequality { - operator: NumericComparison::LessThan, - left: QueryValue::Column(d0v.clone()), - right: QueryValue::TypedValue(TypedValue::Long(10)), - }), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(d0, ValueType::Long)), - ])); - } } diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 8bbc2b42..2c2a57cf 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -370,7 +370,6 @@ mod testing { right: QueryValue::TypedValue(TypedValue::Long(30)), }), ColumnConstraintOrAlternation::Constraint(ColumnConstraint::NotExists(ComputedTable::Subquery(subquery))), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(d0.clone(), ValueType::Long)), ])); assert_eq!(cc.column_bindings.get(&vx), Some(&vec![d0e])); assert_eq!(cc.from, vec![SourceAlias(DatomsTable::Datoms, d0)]); diff --git a/query-algebrizer/src/clauses/or.rs b/query-algebrizer/src/clauses/or.rs index 86b2300d..26ecf48a 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -976,7 +976,6 @@ mod testing { ])), // The outer pattern joins against the `or`. ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(d0e.clone(), QueryValue::Column(d1e.clone()))), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(d0.clone(), ValueType::Long)), ])); assert_eq!(cc.column_bindings.get(&vx), Some(&vec![d0e, d1e])); assert_eq!(cc.from, vec![SourceAlias(DatomsTable::Datoms, d0), diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index 4a0b3554..5b3b0c1f 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -286,7 +286,7 @@ fn test_numeric_less_than_unknown_attribute() { // Although we infer numericness from numeric predicates, we've already assigned a table to the // first pattern, and so this is _still_ `all_datoms`. - assert_eq!(sql, "SELECT DISTINCT `all_datoms00`.e AS `?x` FROM `all_datoms` AS `all_datoms00` WHERE `all_datoms00`.v < 10 AND `all_datoms00`.value_type_tag = 5"); + assert_eq!(sql, "SELECT DISTINCT `all_datoms00`.e AS `?x` FROM `all_datoms` AS `all_datoms00` WHERE `all_datoms00`.v < 10"); assert_eq!(args, vec![]); } @@ -295,7 +295,7 @@ fn test_numeric_gte_known_attribute() { let schema = prepopulated_typed_schema(ValueType::Double); let query = r#"[:find ?x :where [?x :foo/bar ?y] [(>= ?y 12.9)]]"#; let SQLQuery { sql, args } = translate(&schema, query); - assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v >= 12.9 AND `datoms00`.value_type_tag = 5"); + assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v >= 12.9"); assert_eq!(args, vec![]); } @@ -304,7 +304,7 @@ fn test_numeric_not_equals_known_attribute() { let schema = prepopulated_typed_schema(ValueType::Long); let query = r#"[:find ?x . :where [?x :foo/bar ?y] [(!= ?y 12)]]"#; let SQLQuery { sql, args } = translate(&schema, query); - assert_eq!(sql, "SELECT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v <> 12 AND `datoms00`.value_type_tag = 5 LIMIT 1"); + assert_eq!(sql, "SELECT `datoms00`.e AS `?x` FROM `datoms` AS `datoms00` WHERE `datoms00`.a = 99 AND `datoms00`.v <> 12 LIMIT 1"); assert_eq!(args, vec![]); } @@ -657,7 +657,7 @@ fn test_compound_with_ground() { let query = r#"[:find ?x . :where [_ :foo/bar ?x] [(ground "yyy") ?x]]"#; let SQLQuery { sql, args } = translate(&schema, query); assert_eq!(sql, "SELECT $v0 AS `?x` FROM `datoms` AS `datoms00` \ - WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 AND `datoms00`.value_type_tag = 10 LIMIT 1"); + WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 LIMIT 1"); assert_eq!(args, vec![make_arg("$v0", "yyy")]);