From f7851105368512d6b03a1d285e2b49ebf74af483 Mon Sep 17 00:00:00 2001 From: Emily Toop Date: Wed, 14 Jun 2017 14:24:07 +0100 Subject: [PATCH] Call expand_type_tags when algebrizing. * Correct errors and add missing cases * Update tests that now add type tags --- query-algebrizer/src/clauses/mod.rs | 226 ++++++++++++++-------------- query-algebrizer/src/clauses/not.rs | 1 + query-algebrizer/src/clauses/or.rs | 1 + query-algebrizer/src/lib.rs | 1 + query-translator/tests/translate.rs | 8 +- 5 files changed, 117 insertions(+), 120 deletions(-) diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index e7c899e7..f124c5d2 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -837,126 +837,127 @@ impl ConjoiningClauses { } } + 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() { - let cols = self.column_bindings.get(v).unwrap().clone(); - - for col in cols { - match col.1 { - Column::Fixed(DatomsColumn::Value) => { - // attempt to figure out exactly which type is used - 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(ref val) => { - 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 = ValueType::Ref; - if value_type_set.contains(desired) { - match alias.1 { - Column::Fixed(DatomsColumn::Entity) => { - value_type = Some(desired); - }, - _ => { empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(desired)}); }, - } - } else { - empty_reason = Some(EmptyBecause::TypeMismatch{var: v.clone(), existing: value_type_set.clone(), desired: ValueTypeSet::of_one(desired)}); - } - }, - &QueryValue::Entid(ref entid) => (), - } - }, - &ColumnConstraint::NumericInequality { ref 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()) => { + if self.column_bindings.contains_key(v) { + let cols = self.column_bindings.get(v).unwrap().clone(); + 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()); - }, - &QueryValue::PrimitiveLong(ref val) if value_type_set.contains(ValueType::Long) => { - value_type = Some(ValueType::Long); - }, - _ => (), + } 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())}); } - }, - _ => (), - } - }, - &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(ref alternation) => (), + }, + _ => (), + } + }, + &ColumnConstraintOrAlternation::Alternation(_) => (), + } } - } - if value_type.is_some() { - wheres_to_add.push(ColumnConstraintOrAlternation::Constraint(ColumnConstraint::HasType(col.0, value_type.expect("Expected type").clone()))); - } - }, - Column::Fixed(DatomsColumn::Entity) => { - println!("Entity column {:?}", col); - for constraint in self.wheres.0.iter() { - println!("* constraint {:?}", constraint); - match constraint { - &ColumnConstraintOrAlternation::Constraint(ref c) => { - match c { - &ColumnConstraint::HasType(ref alias, ref value_type) => { - println!("hastype {:?}, {:?}, {:?}", alias, value_type, col.0); - 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())}); - } - } - }, - _ => { println!("some other kind of column constraint {:?}", c); } - } - }, - &ColumnConstraintOrAlternation::Alternation(ref alternation) => { - println!("alternation {:?}", 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::Attribute) => { - println!("Attribute column {:?}", col); - }, - _ => { - println!("column {:?}", col); + }, + 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 empty_reason.is_some() { self.mark_known_empty(empty_reason.unwrap()); return; } - self.wheres.0.append(&mut wheres_to_add); } } @@ -1024,7 +1025,6 @@ mod tests { use super::*; - use mentat_core::SQLValueType; use self::mentat_query_parser::parse_find_string; use types::NumericComparison; use algebrize; @@ -1096,8 +1096,7 @@ mod tests { [?x _ ?y] [?a _ ?y]]"#; let parsed = parse_find_string(query).expect("parse failed"); - let mut cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - cc.expand_type_tags(); + let cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; let d0 = "datoms00".to_string(); let d0e = QualifiedAlias::new(d0.clone(), DatomsColumn::Entity); @@ -1131,14 +1130,11 @@ mod tests { :where [?x ?y true] [?z ?y ?x]]"#; let parsed = parse_find_string(query).expect("parse failed"); - let mut cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - - let vx = Variable::from_valid_name("?x"); - assert!(!cc.is_known_empty()); - cc.expand_type_tags(); + 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: vx, existing: ValueTypeSet::of_one(ValueType::Ref), desired: ValueTypeSet::of_one(ValueType::Boolean)}); + assert_eq!(cc.empty_because.unwrap(), EmptyBecause::TypeMismatch{var: vy, existing: ValueTypeSet::of_one(ValueType::Ref), desired: ValueTypeSet::of_one(ValueType::Boolean)}); } #[test] @@ -1150,8 +1146,7 @@ mod tests { [?x _ ?y] [?y _ ?z]]"#; let parsed = parse_find_string(query).expect("parse failed"); - let mut cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - cc.expand_type_tags(); + let cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; println!("{:?}", cc); let d0 = "all_datoms00".to_string(); @@ -1176,8 +1171,7 @@ mod tests { [?x _ ?v] [(< ?v 10)]]"#; let parsed = parse_find_string(query).expect("parse failed"); - let mut cc = algebrize(&schema, parsed).expect("Expected a valid query").cc; - cc.expand_type_tags(); + 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); diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 2c2a57cf..8bbc2b42 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -370,6 +370,7 @@ 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 26ecf48a..86b2300d 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -976,6 +976,7 @@ 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-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 9bf9b8d1..45993b6d 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -183,6 +183,7 @@ pub fn algebrize_with_inputs(schema: &Schema, } cc.expand_column_bindings(); cc.prune_extracted_types(); + cc.expand_type_tags(); let (order, extra_vars) = validate_and_simplify_order(&cc, parsed.order)?; let with: BTreeSet = parsed.with.into_iter().chain(extra_vars.into_iter()).collect(); diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index 5b3b0c1f..4a0b3554 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"); + 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!(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"); + 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!(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 LIMIT 1"); + 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!(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 LIMIT 1"); + WHERE `datoms00`.a = 99 AND `datoms00`.v = $v0 AND `datoms00`.value_type_tag = 10 LIMIT 1"); assert_eq!(args, vec![make_arg("$v0", "yyy")]);