From eaf3e7fc4b1ee6460762bb12d748e968f7a35ebe Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Wed, 14 Jun 2017 16:17:25 -0700 Subject: [PATCH] Extend inequalities to Instants. (#439) r=fluffyemily,nalexander --- query-algebrizer/src/clauses/convert.rs | 47 +++++++ query-algebrizer/src/clauses/not.rs | 6 +- query-algebrizer/src/clauses/or.rs | 6 +- query-algebrizer/src/clauses/predicate.rs | 113 ++++++++++++---- query-algebrizer/src/clauses/resolve.rs | 33 ++++- query-algebrizer/src/errors.rs | 5 + query-algebrizer/src/lib.rs | 5 + query-algebrizer/src/types.rs | 57 ++++++--- query-algebrizer/tests/predicate.rs | 149 ++++++++++++++++++++++ query-translator/src/translate.rs | 2 +- query-translator/tests/translate.rs | 67 ++++++++++ query/src/lib.rs | 9 ++ tests/query.rs | 34 +++++ 13 files changed, 484 insertions(+), 49 deletions(-) create mode 100644 query-algebrizer/tests/predicate.rs diff --git a/query-algebrizer/src/clauses/convert.rs b/query-algebrizer/src/clauses/convert.rs index a9975699..f232ef64 100644 --- a/query-algebrizer/src/clauses/convert.rs +++ b/query-algebrizer/src/clauses/convert.rs @@ -51,6 +51,53 @@ macro_rules! coerce_to_typed_value { } } } +pub trait ValueTypes { + fn potential_types(&self, schema: &Schema) -> Result; +} + +impl ValueTypes for FnArg { + fn potential_types(&self, schema: &Schema) -> Result { + Ok(match self { + &FnArg::EntidOrInteger(x) => { + if ValueType::Ref.accommodates_integer(x) { + // TODO: also see if it's a valid entid? + ValueTypeSet::of_longs() + } else { + ValueTypeSet::of_one(ValueType::Long) + } + }, + + &FnArg::IdentOrKeyword(ref x) => { + if schema.get_entid(x).is_some() { + ValueTypeSet::of_keywords() + } else { + ValueTypeSet::of_one(ValueType::Keyword) + } + }, + + &FnArg::Variable(_) => { + ValueTypeSet::any() + }, + + &FnArg::Constant(NonIntegerConstant::BigInteger(_)) => { + // Not yet implemented. + bail!(ErrorKind::UnsupportedArgument) + }, + + // These don't make sense here. TODO: split FnArg into scalar and non-scalar… + &FnArg::Vector(_) | + &FnArg::SrcVar(_) => bail!(ErrorKind::UnsupportedArgument), + + // These are all straightforward. + &FnArg::Constant(NonIntegerConstant::Boolean(_)) => ValueTypeSet::of_one(ValueType::Boolean), + &FnArg::Constant(NonIntegerConstant::Instant(_)) => ValueTypeSet::of_one(ValueType::Instant), + &FnArg::Constant(NonIntegerConstant::Uuid(_)) => ValueTypeSet::of_one(ValueType::Uuid), + &FnArg::Constant(NonIntegerConstant::Float(_)) => ValueTypeSet::of_one(ValueType::Double), + &FnArg::Constant(NonIntegerConstant::Text(_)) => ValueTypeSet::of_one(ValueType::String), + }) + } +} + pub enum ValueConversion { Val(TypedValue), Impossible(EmptyBecause), diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 2c2a57cf..308cf28a 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -109,7 +109,7 @@ mod testing { ColumnIntersection, DatomsColumn, DatomsTable, - NumericComparison, + Inequality, QualifiedAlias, QueryValue, SourceAlias, @@ -364,8 +364,8 @@ mod testing { assert!(!cc.is_known_empty()); assert_eq!(cc.wheres, ColumnIntersection(vec![ ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(d0a.clone(), age.clone())), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::NumericInequality { - operator: NumericComparison::LessThan, + ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Inequality { + operator: Inequality::LessThan, left: QueryValue::Column(d0v.clone()), right: QueryValue::TypedValue(TypedValue::Long(30)), }), diff --git a/query-algebrizer/src/clauses/or.rs b/query-algebrizer/src/clauses/or.rs index 26ecf48a..ed1cc239 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -751,7 +751,7 @@ mod testing { ColumnConstraint, DatomsColumn, DatomsTable, - NumericComparison, + Inequality, QualifiedAlias, QueryValue, SourceAlias, @@ -960,8 +960,8 @@ mod testing { assert!(!cc.is_known_empty()); assert_eq!(cc.wheres, ColumnIntersection(vec![ ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Equals(d0a.clone(), age.clone())), - ColumnConstraintOrAlternation::Constraint(ColumnConstraint::NumericInequality { - operator: NumericComparison::LessThan, + ColumnConstraintOrAlternation::Constraint(ColumnConstraint::Inequality { + operator: Inequality::LessThan, left: QueryValue::Column(d0v.clone()), right: QueryValue::TypedValue(TypedValue::Long(30)), }), diff --git a/query-algebrizer/src/clauses/predicate.rs b/query-algebrizer/src/clauses/predicate.rs index 16b12f8f..0a77ac21 100644 --- a/query-algebrizer/src/clauses/predicate.rs +++ b/query-algebrizer/src/clauses/predicate.rs @@ -10,14 +10,18 @@ use mentat_core::{ Schema, + ValueType, }; use mentat_query::{ + FnArg, Predicate, }; use clauses::ConjoiningClauses; +use clauses::convert::ValueTypes; + use errors::{ Result, ErrorKind, @@ -25,7 +29,9 @@ use errors::{ use types::{ ColumnConstraint, - NumericComparison, + EmptyBecause, + Inequality, + ValueTypeSet, }; /// Application of predicates. @@ -39,19 +45,25 @@ impl ConjoiningClauses { pub fn apply_predicate<'s>(&mut self, schema: &'s Schema, predicate: Predicate) -> Result<()> { // Because we'll be growing the set of built-in predicates, handling each differently, // and ultimately allowing user-specified predicates, we match on the predicate name first. - if let Some(op) = NumericComparison::from_datalog_operator(predicate.operator.0.as_str()) { - self.apply_numeric_predicate(schema, op, predicate) + if let Some(op) = Inequality::from_datalog_operator(predicate.operator.0.as_str()) { + self.apply_inequality(schema, op, predicate) } else { bail!(ErrorKind::UnknownFunction(predicate.operator.clone())) } } + fn potential_types(&self, schema: &Schema, fn_arg: &FnArg) -> Result { + match fn_arg { + &FnArg::Variable(ref v) => Ok(self.known_type_set(v)), + _ => fn_arg.potential_types(schema), + } + } + /// This function: /// - Resolves variables and converts types to those more amenable to SQL. /// - Ensures that the predicate functions name a known operator. - /// - Accumulates a `NumericInequality` constraint into the `wheres` list. - #[allow(unused_variables)] - pub fn apply_numeric_predicate<'s>(&mut self, schema: &'s Schema, comparison: NumericComparison, predicate: Predicate) -> Result<()> { + /// - Accumulates an `Inequality` constraint into the `wheres` list. + pub fn apply_inequality<'s>(&mut self, schema: &'s Schema, comparison: Inequality, predicate: Predicate) -> Result<()> { if predicate.args.len() != 2 { bail!(ErrorKind::InvalidNumberOfArguments(predicate.operator.clone(), predicate.args.len(), 2)); } @@ -60,21 +72,74 @@ impl ConjoiningClauses { // Any variables that aren't bound by this point in the linear processing of clauses will // cause the application of the predicate to fail. let mut args = predicate.args.into_iter(); - let left = self.resolve_numeric_argument(&predicate.operator, 0, args.next().unwrap())?; - let right = self.resolve_numeric_argument(&predicate.operator, 1, args.next().unwrap())?; + let left = args.next().expect("two args"); + let right = args.next().expect("two args"); - // These arguments must be variables or numeric constants. - // TODO: generalize argument resolution and validation for different kinds of predicates: - // as we process `(< ?x 5)` we are able to check or deduce that `?x` is numeric, and either - // simplify the pattern or optimize the rest of the query. - // To do so needs a slightly more sophisticated representation of type constraints — a set, - // not a single `Option`. + // The types we're handling here must be the intersection of the possible types of the arguments, + // the known types of any variables, and the types supported by our inequality operators. + let supported_types = comparison.supported_types(); + let mut left_types = self.potential_types(schema, &left)? + .intersection(&supported_types); + if left_types.is_empty() { + bail!(ErrorKind::InvalidArgument(predicate.operator.clone(), "numeric or instant", 0)); + } + + let mut right_types = self.potential_types(schema, &right)? + .intersection(&supported_types); + if right_types.is_empty() { + bail!(ErrorKind::InvalidArgument(predicate.operator.clone(), "numeric or instant", 1)); + } + + // We would like to allow longs to compare to doubles. + // Do this by expanding the type sets. `resolve_numeric_argument` will + // use `Long` by preference. + if right_types.contains(ValueType::Long) { + right_types.insert(ValueType::Double); + } + if left_types.contains(ValueType::Long) { + left_types.insert(ValueType::Double); + } + + let shared_types = left_types.intersection(&right_types); + if shared_types.is_empty() { + // In isolation these are both valid inputs to the operator, but the query cannot + // succeed because the types don't match. + self.mark_known_empty( + if let Some(var) = left.as_variable().or_else(|| right.as_variable()) { + EmptyBecause::TypeMismatch { + var: var.clone(), + existing: left_types, + desired: right_types, + } + } else { + EmptyBecause::KnownTypeMismatch { + left: left_types, + right: right_types, + } + }); + return Ok(()); + } + + // We expect the intersection to be Long, Long+Double, Double, or Instant. + let left_v; + let right_v; + if shared_types == ValueTypeSet::of_one(ValueType::Instant) { + left_v = self.resolve_instant_argument(&predicate.operator, 0, left)?; + right_v = self.resolve_instant_argument(&predicate.operator, 1, right)?; + } else if !shared_types.is_empty() && shared_types.is_subset(&ValueTypeSet::of_numeric_types()) { + left_v = self.resolve_numeric_argument(&predicate.operator, 0, left)?; + right_v = self.resolve_numeric_argument(&predicate.operator, 1, right)?; + } else { + bail!(ErrorKind::InvalidArgument(predicate.operator.clone(), "numeric or instant", 0)); + } + + // These arguments must be variables or instant/numeric constants. // TODO: static evaluation. #383. - let constraint = ColumnConstraint::NumericInequality { + let constraint = ColumnConstraint::Inequality { operator: comparison, - left: left, - right: right, + left: left_v, + right: right_v, }; self.wheres.add_intersection(constraint); Ok(()) @@ -119,7 +184,7 @@ mod testing { /// Apply two patterns: a pattern and a numeric predicate. /// Verify that after application of the predicate we know that the value /// must be numeric. - fn test_apply_numeric_predicate() { + fn test_apply_inequality() { let mut cc = ConjoiningClauses::default(); let mut schema = Schema::default(); @@ -141,8 +206,8 @@ mod testing { assert!(!cc.is_known_empty()); let op = PlainSymbol::new("<"); - let comp = NumericComparison::from_datalog_operator(op.plain_name()).unwrap(); - assert!(cc.apply_numeric_predicate(&schema, comp, Predicate { + let comp = Inequality::from_datalog_operator(op.plain_name()).unwrap(); + assert!(cc.apply_inequality(&schema, comp, Predicate { operator: op, args: vec![ FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10), @@ -162,8 +227,8 @@ mod testing { let clauses = cc.wheres; assert_eq!(clauses.len(), 1); - assert_eq!(clauses.0[0], ColumnConstraint::NumericInequality { - operator: NumericComparison::LessThan, + assert_eq!(clauses.0[0], ColumnConstraint::Inequality { + operator: Inequality::LessThan, left: QueryValue::Column(cc.column_bindings.get(&y).unwrap()[0].clone()), right: QueryValue::TypedValue(TypedValue::Long(10)), }.into()); @@ -201,8 +266,8 @@ mod testing { assert!(!cc.is_known_empty()); let op = PlainSymbol::new(">="); - let comp = NumericComparison::from_datalog_operator(op.plain_name()).unwrap(); - assert!(cc.apply_numeric_predicate(&schema, comp, Predicate { + let comp = Inequality::from_datalog_operator(op.plain_name()).unwrap(); + assert!(cc.apply_inequality(&schema, comp, Predicate { operator: op, args: vec![ FnArg::Variable(Variable::from_valid_name("?y")), FnArg::EntidOrInteger(10), diff --git a/query-algebrizer/src/clauses/resolve.rs b/query-algebrizer/src/clauses/resolve.rs index bb8ae199..4abeb5ca 100644 --- a/query-algebrizer/src/clauses/resolve.rs +++ b/query-algebrizer/src/clauses/resolve.rs @@ -10,6 +10,7 @@ use mentat_core::{ TypedValue, + ValueType, }; use mentat_query::{ @@ -55,7 +56,7 @@ impl ConjoiningClauses { Constant(NonIntegerConstant::Boolean(_)) | Constant(NonIntegerConstant::Text(_)) | Constant(NonIntegerConstant::Uuid(_)) | - Constant(NonIntegerConstant::Instant(_)) | // Instants are covered elsewhere. + Constant(NonIntegerConstant::Instant(_)) | // Instants are covered below. Constant(NonIntegerConstant::BigInteger(_)) | Vector(_) => { self.mark_known_empty(EmptyBecause::NonNumericArgument); @@ -65,6 +66,36 @@ impl ConjoiningClauses { } } + /// Just like `resolve_numeric_argument`, but for `ValueType::Instant`. + pub fn resolve_instant_argument(&mut self, function: &PlainSymbol, position: usize, arg: FnArg) -> Result { + 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()))) + }, + Constant(NonIntegerConstant::Instant(v)) => { + Ok(QueryValue::TypedValue(TypedValue::Instant(v))) + }, + + // TODO: should we allow integers if they seem to be timestamps? It's ambiguous… + EntidOrInteger(_) | + IdentOrKeyword(_) | + SrcVar(_) | + Constant(NonIntegerConstant::Boolean(_)) | + Constant(NonIntegerConstant::Float(_)) | + Constant(NonIntegerConstant::Text(_)) | + Constant(NonIntegerConstant::Uuid(_)) | + Constant(NonIntegerConstant::BigInteger(_)) | + Vector(_) => { + self.mark_known_empty(EmptyBecause::NonInstantArgument); + bail!(ErrorKind::InvalidArgument(function.clone(), "instant", position)); + }, + } + } /// Take a function argument and turn it into a `QueryValue` suitable for use in a concrete /// constraint. diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index 65dfcee6..a0607b1a 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -39,6 +39,11 @@ error_chain! { } errors { + UnsupportedArgument { + description("unexpected FnArg") + display("unexpected FnArg") + } + InputTypeDisagreement(var: PlainSymbol, declared: ValueType, provided: ValueType) { description("input type disagreement") display("value of type {} provided for var {}, expected {}", provided, var, declared) diff --git a/query-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 950c488a..b25b221d 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -56,6 +56,11 @@ pub use clauses::{ QueryInputs, }; +pub use types::{ + EmptyBecause, + ValueTypeSet, +}; + #[derive(Debug)] pub struct AlgebraicQuery { default_source: SrcVar, diff --git a/query-algebrizer/src/types.rs b/query-algebrizer/src/types.rs index e9f11e7d..970404a1 100644 --- a/query-algebrizer/src/types.rs +++ b/query-algebrizer/src/types.rs @@ -274,10 +274,11 @@ impl From for OrderBy { } #[derive(Copy, Clone, PartialEq, Eq)] -/// Define the different numeric inequality operators that we support. +/// Define the different inequality operators that we support. /// Note that we deliberately don't just use "<=" and friends as strings: /// Datalog and SQL don't use the same operators (e.g., `<>` and `!=`). -pub enum NumericComparison { +/// These are applicable to numbers and instants. +pub enum Inequality { LessThan, LessThanOrEquals, GreaterThan, @@ -285,9 +286,9 @@ pub enum NumericComparison { NotEquals, } -impl NumericComparison { +impl Inequality { pub fn to_sql_operator(self) -> &'static str { - use self::NumericComparison::*; + use self::Inequality::*; match self { LessThan => "<", LessThanOrEquals => "<=", @@ -297,21 +298,28 @@ impl NumericComparison { } } - pub fn from_datalog_operator(s: &str) -> Option { + pub fn from_datalog_operator(s: &str) -> Option { match s { - "<" => Some(NumericComparison::LessThan), - "<=" => Some(NumericComparison::LessThanOrEquals), - ">" => Some(NumericComparison::GreaterThan), - ">=" => Some(NumericComparison::GreaterThanOrEquals), - "!=" => Some(NumericComparison::NotEquals), + "<" => Some(Inequality::LessThan), + "<=" => Some(Inequality::LessThanOrEquals), + ">" => Some(Inequality::GreaterThan), + ">=" => Some(Inequality::GreaterThanOrEquals), + "!=" => Some(Inequality::NotEquals), _ => None, } } + + // The built-in inequality operators apply to Long, Double, and Instant. + pub fn supported_types(&self) -> ValueTypeSet { + let mut ts = ValueTypeSet::of_numeric_types(); + ts.insert(ValueType::Instant); + ts + } } -impl Debug for NumericComparison { +impl Debug for Inequality { fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { - use self::NumericComparison::*; + use self::Inequality::*; f.write_str(match self { &LessThan => "<", &LessThanOrEquals => "<=", @@ -325,15 +333,13 @@ impl Debug for NumericComparison { #[derive(PartialEq, Eq)] pub enum ColumnConstraint { Equals(QualifiedAlias, QueryValue), - NumericInequality { - operator: NumericComparison, + Inequality { + operator: Inequality, left: QueryValue, right: QueryValue, }, HasType(TableAlias, ValueType), NotExists(ComputedTable), - // TODO: Merge this with NumericInequality? I expect the fine-grained information to be - // valuable when optimizing. Matches(QualifiedAlias, QueryValue), } @@ -441,7 +447,7 @@ impl Debug for ColumnConstraint { write!(f, "{:?} = {:?}", qa1, thing) }, - &NumericInequality { operator, ref left, ref right } => { + &Inequality { operator, ref left, ref right } => { write!(f, "{:?} {:?} {:?}", left, operator, right) }, @@ -462,9 +468,15 @@ impl Debug for ColumnConstraint { #[derive(PartialEq, Clone)] pub enum EmptyBecause { ConflictingBindings { var: Variable, existing: TypedValue, desired: TypedValue }, + + // A variable is known to be of two conflicting sets of types. TypeMismatch { var: Variable, existing: ValueTypeSet, desired: ValueTypeSet }, + + // The same, but for non-variables. + KnownTypeMismatch { left: ValueTypeSet, right: ValueTypeSet }, NoValidTypes(Variable), NonAttributeArgument, + NonInstantArgument, NonNumericArgument, NonStringFulltextValue, UnresolvedIdent(NamespacedKeyword), @@ -487,12 +499,19 @@ impl Debug for EmptyBecause { write!(f, "Type mismatch: {:?} can't be {:?}, because it's already {:?}", var, desired, existing) }, + &KnownTypeMismatch { ref left, ref right } => { + write!(f, "Type mismatch: {:?} can't be compared to {:?}", + left, right) + }, &NoValidTypes(ref var) => { write!(f, "Type mismatch: {:?} has no valid types", var) }, &NonAttributeArgument => { write!(f, "Non-attribute argument in attribute place") }, + &NonInstantArgument => { + write!(f, "Non-instant argument in instant place") + }, &NonNumericArgument => { write!(f, "Non-numeric argument in numeric place") }, @@ -612,6 +631,10 @@ impl ValueTypeSet { self.0.iter().next() } + pub fn is_subset(&self, other: &ValueTypeSet) -> bool { + self.0.is_subset(&other.0) + } + pub fn contains(&self, vt: ValueType) -> bool { self.0.contains(&vt) } diff --git a/query-algebrizer/tests/predicate.rs b/query-algebrizer/tests/predicate.rs new file mode 100644 index 00000000..a069819b --- /dev/null +++ b/query-algebrizer/tests/predicate.rs @@ -0,0 +1,149 @@ +// Copyright 2016 Mozilla +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. + +extern crate mentat_core; +extern crate mentat_query; +extern crate mentat_query_algebrizer; +extern crate mentat_query_parser; + +use std::collections::BTreeMap; + +use mentat_core::{ + Attribute, + Entid, + Schema, + ValueType, + TypedValue, +}; + +use mentat_query_parser::{ + parse_find_string, +}; + +use mentat_query::{ + NamespacedKeyword, + PlainSymbol, + Variable, +}; + +use mentat_query_algebrizer::{ + BindingError, + ConjoiningClauses, + ComputedTable, + EmptyBecause, + Error, + ErrorKind, + QueryInputs, + ValueTypeSet, + algebrize, + algebrize_with_inputs, +}; + +// These are helpers that tests use to build Schema instances. +#[cfg(test)] +fn associate_ident(schema: &mut Schema, i: NamespacedKeyword, e: Entid) { + schema.entid_map.insert(e, i.clone()); + schema.ident_map.insert(i.clone(), e); +} + +#[cfg(test)] +fn add_attribute(schema: &mut Schema, e: Entid, a: Attribute) { + schema.schema_map.insert(e, a); +} + +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); + add_attribute(&mut schema, 65, Attribute { + value_type: ValueType::Instant, + multival: false, + ..Default::default() + }); + add_attribute(&mut schema, 66, Attribute { + value_type: ValueType::Double, + multival: false, + ..Default::default() + }); + schema +} + +fn bails(schema: &Schema, input: &str) -> Error { + let parsed = parse_find_string(input).expect("query input to have parsed"); + algebrize(schema.into(), parsed).expect_err("algebrize to have failed") +} + +fn bails_with_inputs(schema: &Schema, input: &str, inputs: QueryInputs) -> Error { + let parsed = parse_find_string(input).expect("query input to have parsed"); + algebrize_with_inputs(schema, parsed, 0, inputs).expect_err("algebrize to have failed") +} + +fn alg(schema: &Schema, input: &str) -> ConjoiningClauses { + let parsed = parse_find_string(input).expect("query input to have parsed"); + algebrize(schema.into(), parsed).expect("algebrizing to have succeeded").cc +} + +#[test] +fn test_instant_predicates_require_instants() { + let schema = prepopulated_schema(); + + // You can't use a string for an inequality: this is a straight-up error. + let query = r#"[:find ?e + :where + [?e :foo/date ?t] + [(> ?t "2017-06-16T00:56:41.257Z")]]"#; + match bails(&schema, query).0 { + ErrorKind::InvalidArgument(op, why, idx) => { + assert_eq!(op, PlainSymbol::new(">")); + assert_eq!(why, "numeric or instant"); + assert_eq!(idx, 1); + }, + _ => panic!("Expected InvalidArgument."), + } + + let query = r#"[:find ?e + :where + [?e :foo/date ?t] + [(> "2017-06-16T00:56:41.257Z", ?t)]]"#; + match bails(&schema, query).0 { + ErrorKind::InvalidArgument(op, why, idx) => { + assert_eq!(op, PlainSymbol::new(">")); + assert_eq!(why, "numeric or instant"); + assert_eq!(idx, 0); // We get this right. + }, + _ => panic!("Expected InvalidArgument."), + } + + // You can try using a number, which is valid input to a numeric predicate. + // In this store and query, though, that means we expect `?t` to be both + // an instant and a number, so the query is known-empty. + let query = r#"[:find ?e + :where + [?e :foo/date ?t] + [(> ?t 1234512345)]]"#; + let cc = alg(&schema, query); + assert!(cc.is_known_empty()); + assert_eq!(cc.empty_because.unwrap(), + EmptyBecause::TypeMismatch { + var: Variable::from_valid_name("?t"), + existing: ValueTypeSet::of_one(ValueType::Instant), + desired: ValueTypeSet::of_numeric_types(), + }); + + // You can compare doubles to longs. + let query = r#"[:find ?e + :where + [?e :foo/double ?t] + [(< ?t 1234512345)]]"#; + let cc = alg(&schema, query); + assert!(!cc.is_known_empty()); + assert_eq!(cc.known_type(&Variable::from_valid_name("?t")).expect("?t is known"), + ValueType::Double); +} diff --git a/query-translator/src/translate.rs b/query-translator/src/translate.rs index af3ff6b5..ae19d786 100644 --- a/query-translator/src/translate.rs +++ b/query-translator/src/translate.rs @@ -140,7 +140,7 @@ impl ToConstraint for ColumnConstraint { } }, - NumericInequality { operator, left, right } => { + Inequality { operator, left, right } => { Constraint::Infix { op: Op(operator.to_sql_operator()), left: left.into(), diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index 66face17..fb9d0aec 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -317,6 +317,56 @@ fn test_numeric_not_equals_known_attribute() { assert_eq!(args, vec![]); } +#[test] +fn test_compare_long_to_double_constants() { + let schema = prepopulated_typed_schema(ValueType::Double); + + let query = r#"[:find ?e . + :where + [?e :foo/bar ?v] + [(< 99.0 1234512345)]]"#; + let SQLQuery { sql, args } = translate(&schema, query); + assert_eq!(sql, "SELECT `datoms00`.e AS `?e` FROM `datoms` AS `datoms00` \ + WHERE `datoms00`.a = 99 \ + AND 9.9e1 < 1234512345 \ + LIMIT 1"); + assert_eq!(args, vec![]); +} + +#[test] +fn test_compare_long_to_double() { + let schema = prepopulated_typed_schema(ValueType::Double); + + // You can compare longs to doubles. + let query = r#"[:find ?e . + :where + [?e :foo/bar ?t] + [(< ?t 1234512345)]]"#; + let SQLQuery { sql, args } = translate(&schema, query); + assert_eq!(sql, "SELECT `datoms00`.e AS `?e` FROM `datoms` AS `datoms00` \ + WHERE `datoms00`.a = 99 \ + AND `datoms00`.v < 1234512345 \ + LIMIT 1"); + assert_eq!(args, vec![]); +} + +#[test] +fn test_compare_double_to_long() { + let schema = prepopulated_typed_schema(ValueType::Long); + + // You can compare doubles to longs. + let query = r#"[:find ?e . + :where + [?e :foo/bar ?t] + [(< ?t 1234512345.0)]]"#; + let SQLQuery { sql, args } = translate(&schema, query); + assert_eq!(sql, "SELECT `datoms00`.e AS `?e` FROM `datoms` AS `datoms00` \ + WHERE `datoms00`.a = 99 \ + AND `datoms00`.v < 1.234512345e9 \ + LIMIT 1"); + assert_eq!(args, vec![]); +} + #[test] fn test_simple_or_join() { let mut schema = Schema::default(); @@ -888,3 +938,20 @@ fn test_fulltext_inputs() { AND `datoms02`.a = 99"); assert_eq!(args, vec![make_arg("$v0", "hello"),]); } + +#[test] +fn test_instant_range() { + let schema = prepopulated_typed_schema(ValueType::Instant); + let query = r#"[:find ?e + :where + [?e :foo/bar ?t] + [(> ?t #inst "2017-06-16T00:56:41.257Z")]]"#; + + let SQLQuery { sql, args } = translate(&schema, query); + assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?e` \ + FROM \ + `datoms` AS `datoms00` \ + WHERE `datoms00`.a = 99 \ + AND `datoms00`.v > 1497574601257000"); + assert_eq!(args, vec![]); +} diff --git a/query/src/lib.rs b/query/src/lib.rs index c412baac..a146a9aa 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -259,6 +259,15 @@ impl FromValue for FnArg { } } +impl FnArg { + pub fn as_variable(&self) -> Option<&Variable> { + match self { + &FnArg::Variable(ref v) => Some(v), + _ => None, + } + } +} + /// e, a, tx can't be values -- no strings, no floats -- and so /// they can only be variables, entity IDs, ident keywords, or /// placeholders. diff --git a/tests/query.rs b/tests/query.rs index 4685834f..9a1f73e9 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -356,3 +356,37 @@ fn test_fulltext() { _ => panic!("Expected query to work."), } } + +#[test] +fn test_instant_range_query() { + let mut c = new_connection("").expect("Couldn't open conn."); + let mut conn = Conn::connect(&mut c).expect("Couldn't open DB."); + + conn.transact(&mut c, r#"[ + [:db/add "a" :db/ident :foo/date] + [:db/add "a" :db/valueType :db.type/instant] + [:db/add "a" :db/cardinality :db.cardinality/one] + ]"#).unwrap(); + + let ids = conn.transact(&mut c, r#"[ + [:db/add "b" :foo/date #inst "2016-01-01T11:00:00.000Z"] + [:db/add "c" :foo/date #inst "2016-06-01T11:00:01.000Z"] + [:db/add "d" :foo/date #inst "2017-01-01T11:00:02.000Z"] + [:db/add "e" :foo/date #inst "2017-06-01T11:00:03.000Z"] + ]"#).unwrap().tempids; + + let r = conn.q_once(&mut c, + r#"[:find [?x ...] + :order (asc ?date) + :where + [?x :foo/date ?date] + [(< ?date #inst "2017-01-01T11:00:02.000Z")]]"#, None); + match r { + Result::Ok(QueryResults::Coll(vals)) => { + assert_eq!(vals, + vec![TypedValue::Ref(*ids.get("b").unwrap()), + TypedValue::Ref(*ids.get("c").unwrap())]); + }, + _ => panic!("Expected query to work."), + } +}