From 5fbf17f4f995aba76a8ebf612a60473d808daa80 Mon Sep 17 00:00:00 2001 From: Emily Toop Date: Thu, 12 Apr 2018 09:42:55 +0100 Subject: [PATCH] Fix variables in predicates --- query-algebrizer/src/clauses/resolve.rs | 47 ++++++++++++----- query-algebrizer/tests/predicate.rs | 68 +++++++++++++++++++++++++ query-algebrizer/tests/utils/mod.rs | 5 ++ 3 files changed, 106 insertions(+), 14 deletions(-) diff --git a/query-algebrizer/src/clauses/resolve.rs b/query-algebrizer/src/clauses/resolve.rs index c3169580..1b76a359 100644 --- a/query-algebrizer/src/clauses/resolve.rs +++ b/query-algebrizer/src/clauses/resolve.rs @@ -45,11 +45,18 @@ impl ConjoiningClauses { use self::FnArg::*; match arg { FnArg::Variable(var) => { - self.constrain_var_to_numeric(var.clone()); - self.column_bindings - .get(&var) - .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + match self.bound_value(&var) { + // The type is already known if it's a bound variable…. + Some(TypedValue::Long(v)) => Ok(QueryValue::TypedValue(TypedValue::Long(v))), + Some(TypedValue::Double(v)) => Ok(QueryValue::TypedValue(TypedValue::Double(v))), + _ => { + self.constrain_var_to_numeric(var.clone()); + self.column_bindings + .get(&var) + .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) + .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + }, + } }, // Can't be an entid. EntidOrInteger(i) => Ok(QueryValue::TypedValue(TypedValue::Long(i))), @@ -73,11 +80,17 @@ impl ConjoiningClauses { use self::FnArg::*; match arg { FnArg::Variable(var) => { - self.constrain_var_to_type(var.clone(), ValueType::Instant); - self.column_bindings - .get(&var) - .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + match self.bound_value(&var) { + // The type is already known if it's a bound variable…. + Some(TypedValue::Instant(v)) => Ok(QueryValue::TypedValue(TypedValue::Instant(v))), + _ => { + self.constrain_var_to_type(var.clone(), ValueType::Instant); + self.column_bindings + .get(&var) + .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) + .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + }, + } }, Constant(NonIntegerConstant::Instant(v)) => { Ok(QueryValue::TypedValue(TypedValue::Instant(v))) @@ -144,10 +157,16 @@ impl ConjoiningClauses { use self::FnArg::*; match arg { FnArg::Variable(var) => { - self.column_bindings - .get(&var) - .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + match self.bound_value(&var) { + // The type is already known if it's a bound variable…. + Some(v) => Ok(QueryValue::TypedValue(v)), + None => { + self.column_bindings + .get(&var) + .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) + .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + }, + } }, EntidOrInteger(i) => Ok(QueryValue::PrimitiveLong(i)), IdentOrKeyword(_) => unimplemented!(), // TODO diff --git a/query-algebrizer/tests/predicate.rs b/query-algebrizer/tests/predicate.rs index 53b0dd6d..42d4e25e 100644 --- a/query-algebrizer/tests/predicate.rs +++ b/query-algebrizer/tests/predicate.rs @@ -17,7 +17,10 @@ mod utils; use mentat_core::{ Attribute, + DateTime, Schema, + TypedValue, + Utc, ValueType, ValueTypeSet, }; @@ -32,11 +35,13 @@ use mentat_query_algebrizer::{ EmptyBecause, ErrorKind, Known, + QueryInputs, }; use utils::{ add_attribute, alg, + alg_with_inputs, associate_ident, bails, }; @@ -45,6 +50,7 @@ fn prepopulated_schema() -> Schema { let mut schema = Schema::default(); associate_ident(&mut schema, NamespacedKeyword::new("foo", "date"), 65); associate_ident(&mut schema, NamespacedKeyword::new("foo", "double"), 66); + associate_ident(&mut schema, NamespacedKeyword::new("foo", "long"), 67); add_attribute(&mut schema, 65, Attribute { value_type: ValueType::Instant, multival: false, @@ -55,6 +61,11 @@ fn prepopulated_schema() -> Schema { multival: false, ..Default::default() }); + add_attribute(&mut schema, 67, Attribute { + value_type: ValueType::Long, + multival: false, + ..Default::default() + }); schema } @@ -116,3 +127,60 @@ fn test_instant_predicates_require_instants() { assert_eq!(cc.known_type(&Variable::from_valid_name("?t")).expect("?t is known"), ValueType::Double); } + +#[test] +fn test_instant_predicates_accepts_var() { + let schema = prepopulated_schema(); + let known = Known::for_schema(&schema); + + let instant_var = Variable::from_valid_name("?time"); + let instant_value = TypedValue::Instant(DateTime::parse_from_rfc3339("2018-04-11T19:17:00.000Z") + .map(|t| t.with_timezone(&Utc)) + .expect("expected valid date")); + + let query = r#"[:find ?e + :in ?time + :where + [?e :foo/date ?t] + [(< ?t ?time)]]"#; + let cc = alg_with_inputs(known, query, QueryInputs::with_value_sequence(vec![(instant_var.clone(), instant_value.clone())])); + assert_eq!(cc.known_type(&instant_var).expect("?time is known"), + ValueType::Instant); + + let query = r#"[:find ?e + :in ?time + :where + [?e :foo/date ?t] + [(> ?time, ?t)]]"#; + let cc = alg_with_inputs(known, query, QueryInputs::with_value_sequence(vec![(instant_var.clone(), instant_value.clone())])); + assert_eq!(cc.known_type(&instant_var).expect("?time is known"), + ValueType::Instant); +} + +#[test] +fn test_numeric_predicates_accepts_var() { + let schema = prepopulated_schema(); + let known = Known::for_schema(&schema); + + let numeric_var = Variable::from_valid_name("?long"); + let numeric_value = TypedValue::Long(1234567); + + // You can't use a string for an inequality: this is a straight-up error. + let query = r#"[:find ?e + :in ?long + :where + [?e :foo/long ?t] + [(> ?t ?long)]]"#; + let cc = alg_with_inputs(known, query, QueryInputs::with_value_sequence(vec![(numeric_var.clone(), numeric_value.clone())])); + assert_eq!(cc.known_type(&numeric_var).expect("?long is known"), + ValueType::Long); + + let query = r#"[:find ?e + :in ?long + :where + [?e :foo/long ?t] + [(> ?long, ?t)]]"#; + let cc = alg_with_inputs(known, query, QueryInputs::with_value_sequence(vec![(numeric_var.clone(), numeric_value.clone())])); + assert_eq!(cc.known_type(&numeric_var).expect("?long is known"), + ValueType::Long); +} diff --git a/query-algebrizer/tests/utils/mod.rs b/query-algebrizer/tests/utils/mod.rs index a0b0e065..35864beb 100644 --- a/query-algebrizer/tests/utils/mod.rs +++ b/query-algebrizer/tests/utils/mod.rs @@ -98,3 +98,8 @@ pub fn alg(known: Known, input: &str) -> ConjoiningClauses { let parsed = parse_find_string(input).expect("query input to have parsed"); algebrize(known, parsed).expect("algebrizing to have succeeded").cc } + +pub fn alg_with_inputs(known: Known, input: &str, inputs: QueryInputs) -> ConjoiningClauses { + let parsed = parse_find_string(input).expect("query input to have parsed"); + algebrize_with_inputs(known, parsed, 0, inputs).expect("algebrizing to have succeeded").cc +}