diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index a3a00589..e51c36bf 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -940,27 +940,39 @@ impl ConjoiningClauses { } } + /// When we're done with all patterns, we might have a set of type requirements that will + /// be used to add additional constraints to the execution plan. + /// + /// This function does so. + /// + /// Furthermore, those type requirements will not yet be present in `known_types`, which + /// means they won't be used by the projector or translator. + /// + /// This step also updates `known_types` to match. pub(crate) fn process_required_types(&mut self) -> Result<()> { if self.empty_because.is_some() { return Ok(()) } + // We can't call `mark_known_empty` inside the loop since it would be a - // mutable borrow on self while we're iterating over `self.required_types`. - // Doing it like this avoids needing to copy `self.required_types`. + // mutable borrow on self while we're using fields on `self`. + // We still need to clone `required_types` 'cos we're mutating in + // `narrow_types_for_var`. let mut empty_because: Option = None; - for (var, types) in self.required_types.iter() { - if let Some(already_known) = self.known_types.get(var) { - if already_known.is_disjoint(types) { + for (var, types) in self.required_types.clone().into_iter() { + if let Some(already_known) = self.known_types.get(&var) { + if already_known.is_disjoint(&types) { // If we know the constraint can't be one of the types // the variable could take, then we know we're empty. empty_because = Some(EmptyBecause::TypeMismatch { - var: var.clone(), + var: var, existing: *already_known, - desired: *types, + desired: types, }); break; } - if already_known.is_subset(types) { + + if already_known.is_subset(&types) { // TODO: I'm not convinced that we can do nothing here. // // Consider `[:find ?x ?v :where [_ _ ?v] [(> ?v 10)] [?x :foo/long ?v]]`. @@ -985,18 +997,23 @@ impl ConjoiningClauses { } } + // Update known types. + self.narrow_types_for_var(var.clone(), types); + let qa = self.extracted_types .get(&var) .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name())))?; self.wheres.add_intersection(ColumnConstraint::HasTypes { value: qa.0.clone(), - value_types: *types, + value_types: types, check_value: true, }); } + if let Some(reason) = empty_because { self.mark_known_empty(reason); } + Ok(()) } diff --git a/tests/query.rs b/tests/query.rs index 044c7658..3757f75b 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -33,6 +33,11 @@ use mentat_core::{ Utc, Uuid, ValueType, + ValueTypeSet, +}; + +use mentat_query_projector::{ + SimpleAggregationOp, }; use mentat::{ @@ -529,6 +534,133 @@ fn test_lookup() { let fetched_many = conn.lookup_value_for_attribute(&c, *entid, &foo_many).unwrap().unwrap(); assert!(two_longs.contains(&fetched_many)); } + +#[test] +fn test_aggregates_type_handling() { + let mut store = Store::open("").expect("opened"); + store.transact(r#"[ + {:db/ident :test/boolean :db/valueType :db.type/boolean :db/cardinality :db.cardinality/one} + {:db/ident :test/long :db/valueType :db.type/long :db/cardinality :db.cardinality/one} + {:db/ident :test/double :db/valueType :db.type/double :db/cardinality :db.cardinality/one} + {:db/ident :test/string :db/valueType :db.type/string :db/cardinality :db.cardinality/one} + {:db/ident :test/keyword :db/valueType :db.type/keyword :db/cardinality :db.cardinality/one} + {:db/ident :test/uuid :db/valueType :db.type/uuid :db/cardinality :db.cardinality/one} + {:db/ident :test/instant :db/valueType :db.type/instant :db/cardinality :db.cardinality/one} + {:db/ident :test/ref :db/valueType :db.type/ref :db/cardinality :db.cardinality/one} + ]"#).unwrap(); + + store.transact(r#"[ + {:test/boolean false + :test/long 10 + :test/double 2.4 + :test/string "one" + :test/keyword :foo/bar + :test/uuid #uuid "55555234-1234-1234-1234-123412341234" + :test/instant #inst "2017-01-01T11:00:00.000Z" + :test/ref 1} + {:test/boolean true + :test/long 20 + :test/double 4.4 + :test/string "two" + :test/keyword :foo/baz + :test/uuid #uuid "66666234-1234-1234-1234-123412341234" + :test/instant #inst "2018-01-01T11:00:00.000Z" + :test/ref 2} + {:test/boolean true + :test/long 30 + :test/double 6.4 + :test/string "three" + :test/keyword :foo/noo + :test/uuid #uuid "77777234-1234-1234-1234-123412341234" + :test/instant #inst "2019-01-01T11:00:00.000Z" + :test/ref 3} + ]"#).unwrap(); + + // No type limits => can't do it. + let r = store.q_once(r#"[:find (sum ?v) . :where [_ _ ?v]]"#, None); + let all_types = ValueTypeSet::any(); + match r { + Result::Err( + Error( + ErrorKind::TranslatorError( + ::mentat_query_translator::ErrorKind::ProjectorError( + ::mentat_query_projector::ErrorKind::CannotApplyAggregateOperationToTypes( + SimpleAggregationOp::Sum, + types + ), + ) + ), + _)) => { + assert_eq!(types, all_types); + }, + r => panic!("Unexpected: {:?}", r), + } + + // You can't sum instants. + let r = store.q_once(r#"[:find (sum ?v) . + :where [_ _ ?v] [(instant ?v)]]"#, + None); + match r { + Result::Err( + Error( + ErrorKind::TranslatorError( + ::mentat_query_translator::ErrorKind::ProjectorError( + ::mentat_query_projector::ErrorKind::CannotApplyAggregateOperationToTypes( + SimpleAggregationOp::Sum, + types + ), + ) + ), + _)) => { + assert_eq!(types, ValueTypeSet::of_one(ValueType::Instant)); + }, + r => panic!("Unexpected: {:?}", r), + } + + // But you can count them. + let r = store.q_once(r#"[:find (count ?v) . + :where [_ _ ?v] [(instant ?v)]]"#, + None) + .into_scalar_result() + .expect("results") + .unwrap(); + + // Our two transactions, the bootstrap transaction, plus the three values. + assert_eq!(TypedValue::Long(6), r); + + // And you can min them, which returns an instant. + let r = store.q_once(r#"[:find (min ?v) . + :where [_ _ ?v] [(instant ?v)]]"#, + None) + .into_scalar_result() + .expect("results") + .unwrap(); + + let earliest = DateTime::parse_from_rfc3339("2017-01-01T11:00:00.000Z").unwrap().with_timezone(&Utc); + assert_eq!(TypedValue::Instant(earliest), r); + + let r = store.q_once(r#"[:find (sum ?v) . + :where [_ _ ?v] [(long ?v)]]"#, + None) + .into_scalar_result() + .expect("results") + .unwrap(); + + // Yes, the current version is in the store as a Long! + let total = 30i64 + 20i64 + 10i64 + ::mentat_db::db::CURRENT_VERSION as i64; + assert_eq!(TypedValue::Long(total), r); + + let r = store.q_once(r#"[:find (avg ?v) . + :where [_ _ ?v] [(double ?v)]]"#, + None) + .into_scalar_result() + .expect("results") + .unwrap(); + + let avg = (6.4f64 / 3f64) + (4.4f64 / 3f64) + (2.4f64 / 3f64); + assert_eq!(TypedValue::Double(avg.into()), r); +} + #[test] fn test_type_reqs() { let mut c = new_connection("").expect("Couldn't open conn.");