From 7a378ebd96feeae22048c401827b23adf8d157c9 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 7 Oct 2015 11:28:35 -0600 Subject: [PATCH] Expand the query DSL to allow `filter` on more than just tables The main thing that stood out to me was that the final test, `filter_then_select` didn't immedaitely pass once I made it compile. It was ignoring the where clause of the parent source. This re-affirms to me that I need to start looking for a more generic way to implement this, instead of having all of these different concrete types. In the short term, I've removed the default impl to force myself to consider the implications of this method on the rest of the types. For Join sources, I think it still makes sense to always return `None`, since they only work with tables at the moment. It's worth noting this now allows chaining `select` and `filter`, where subsequent calls will override the previous. I think that's fine for `select`, but `filter` should of course chain with `and`. This has a lot of the same caveats that we've seen in the past when moving things off of `Table` and onto `QuerySource`. The tests get a tad bit verbose here, but I'm not sure if that's indicative of a problem. Once again I need to do the selectable expression workaround. This can go away once https://github.com/rust-lang/rfcs/pull/1268 lands. I was unable to add a blanket impl in these cases, since it overlapped with the blanket impls defined for types like `expression::Eq`. This was actually quite surprising to me, since both impls had concrete types which could be known for a fact not to overlap. --- src/macros.rs | 18 ++++++++++ src/query_source/joins.rs | 8 +++++ src/query_source/mod.rs | 21 +++++------ src/query_source/select.rs | 4 +++ tests/filter.rs | 71 +++++++++++++++++++++++++++++++++----- 5 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 070a11fe4269..c0911b5f6ac0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -42,6 +42,10 @@ macro_rules! table { fn from_clause(&self) -> String { stringify!($name).to_string() } + + fn where_clause(&self) -> Option<(String, Vec>>)> { + None + } } impl Table for table { @@ -213,5 +217,19 @@ macro_rules! select_column_inner { > for $parent::$column_name { } + + impl $crate::expression::SelectableExpression< + $crate::query_source::SelectSqlQuerySource> + for $parent::$column_name where + $parent::$column_name: $crate::expression::SelectableExpression, + { + } + + impl $crate::expression::SelectableExpression< + $crate::query_source::FilteredQuerySource> + for $parent::$column_name where + $parent::$column_name: $crate::expression::SelectableExpression, + { + } } } diff --git a/src/query_source/joins.rs b/src/query_source/joins.rs index 0edc10a132fe..dd62c418264a 100644 --- a/src/query_source/joins.rs +++ b/src/query_source/joins.rs @@ -30,6 +30,10 @@ impl QuerySource for InnerJoinSource where format!("{} INNER JOIN {} ON {}", self.left.name(), self.right.name(), self.left.join_sql()) } + + fn where_clause(&self) -> Option<(String, Vec>>)> { + None + } } #[derive(Clone, Copy)] @@ -61,6 +65,10 @@ impl QuerySource for LeftOuterJoinSource where format!("{} LEFT OUTER JOIN {} ON {}", self.left.name(), self.right.name(), self.left.join_sql()) } + + fn where_clause(&self) -> Option<(String, Vec>>)> { + None + } } pub trait JoinTo: Table { diff --git a/src/query_source/mod.rs b/src/query_source/mod.rs index 8a4b268bcc29..68a7774ed1cb 100644 --- a/src/query_source/mod.rs +++ b/src/query_source/mod.rs @@ -4,9 +4,9 @@ mod select; use expression::{Expression, SelectableExpression, NonAggregate, SqlLiteral}; use expression::count::*; -use self::filter::FilteredQuerySource; +pub use self::filter::FilteredQuerySource; pub use self::joins::{InnerJoinSource, LeftOuterJoinSource}; -use self::select::SelectSqlQuerySource; +pub use self::select::SelectSqlQuerySource; use std::convert::Into; use types::{self, FromSqlRow, NativeSqlType}; @@ -23,10 +23,7 @@ pub trait QuerySource: Sized { fn select_clause(&self) -> String; fn from_clause(&self) -> String; - - fn where_clause(&self) -> Option<(String, Vec>>)> { - None - } + fn where_clause(&self) -> Option<(String, Vec>>)>; fn select(self, expr: E) -> SelectSqlQuerySource where SelectSqlQuerySource: QuerySource, @@ -52,6 +49,12 @@ pub trait QuerySource: Sized { let sql = SqlLiteral::new(columns.into()); SelectSqlQuerySource::new(sql, self) } + + fn filter(self, predicate: T) -> FilteredQuerySource where + T: SelectableExpression, + { + FilteredQuerySource::new(self, predicate) + } } pub trait Column { @@ -95,10 +98,4 @@ pub trait Table: QuerySource { { LeftOuterJoinSource::new(self, other) } - - fn filter(self, predicate: T) -> FilteredQuerySource where - T: SelectableExpression, - { - FilteredQuerySource::new(self, predicate) - } } diff --git a/src/query_source/select.rs b/src/query_source/select.rs index eeabab586644..d7091c38552c 100644 --- a/src/query_source/select.rs +++ b/src/query_source/select.rs @@ -35,4 +35,8 @@ impl QuerySource for SelectSqlQuerySource where fn from_clause(&self) -> String { self.source.from_clause() } + + fn where_clause(&self) -> Option<(String, Vec>>)> { + self.source.where_clause() + } } diff --git a/tests/filter.rs b/tests/filter.rs index 6d476084f76a..4c3b20e3e43b 100644 --- a/tests/filter.rs +++ b/tests/filter.rs @@ -5,10 +5,7 @@ use yaqb::*; fn filter_by_int_equality() { use schema::users::dsl::*; - let connection = connection(); - setup_users_table(&connection); - let data = [NewUser::new("Sean", None), NewUser::new("Tess", None)]; - connection.insert_without_return(&users, &data).unwrap(); + let connection = connection_with_sean_and_tess_in_users_table(); let sean = User::new(1, "Sean"); let tess = User::new(2, "Tess"); @@ -21,10 +18,7 @@ fn filter_by_int_equality() { fn filter_by_string_equality() { use schema::users::dsl::*; - let connection = connection(); - setup_users_table(&connection); - let data = [NewUser::new("Sean", None), NewUser::new("Tess", None)]; - connection.insert_without_return(&users, &data).unwrap(); + let connection = connection_with_sean_and_tess_in_users_table(); let sean = User::new(1, "Sean"); let tess = User::new(2, "Tess"); @@ -32,3 +26,64 @@ fn filter_by_string_equality() { assert_eq!(Some(tess), connection.query_one(&users.filter(name.eq("Tess"))).unwrap()); assert_eq!(None::, connection.query_one(&users.filter(name.eq("Jim"))).unwrap()); } + +#[test] +fn filter_after_joining() { + use schema::users::name; + + let connection = connection_with_sean_and_tess_in_users_table(); + setup_posts_table(&connection); + connection.execute("INSERT INTO POSTS (title, user_id) VALUES ('Hello', 1), ('World', 2)") + .unwrap(); + + let sean = User::new(1, "Sean"); + let tess = User::new(2, "Tess"); + let seans_post = Post::new(1, 1, "Hello", None); + let tess_post = Post::new(2, 2, "World", None); + let source = users::table.inner_join(posts::table); + assert_eq!(Some((sean, seans_post)), + connection.query_one(&source.filter(name.eq("Sean"))).unwrap()); + assert_eq!(Some((tess, tess_post)), + connection.query_one(&source.filter(name.eq("Tess"))).unwrap()); + assert_eq!(None::<(User, Post)>, + connection.query_one(&source.filter(name.eq("Jim"))).unwrap()); +} + +#[test] +fn select_then_filter() { + use schema::users::dsl::*; + + let connection = connection_with_sean_and_tess_in_users_table(); + + let source = users.select(name); + assert_eq!(Some("Sean".to_string()), + connection.query_one(&source.filter(name.eq("Sean"))).unwrap()); + assert_eq!(Some("Tess".to_string()), + connection.query_one(&source.filter(name.eq("Tess"))).unwrap()); + assert_eq!(None::, connection.query_one(&source.filter(name.eq("Jim"))).unwrap()); +} + +#[test] +fn filter_then_select() { + use schema::users::dsl::*; + + let connection = connection(); + setup_users_table(&connection); + let data = [NewUser::new("Sean", None), NewUser::new("Tess", None)]; + connection.insert_without_return(&users, &data).unwrap(); + + assert_eq!(Some("Sean".to_string()), + connection.query_one(&users.filter(name.eq("Sean")).select(name)).unwrap()); + assert_eq!(Some("Tess".to_string()), + connection.query_one(&users.filter(name.eq("Tess")).select(name)).unwrap()); + assert_eq!(None::, connection.query_one(&users.filter(name.eq("Jim")).select(name)).unwrap()); +} + +fn connection_with_sean_and_tess_in_users_table() -> Connection { + let connection = connection(); + setup_users_table(&connection); + let data = [NewUser::new("Sean", None), NewUser::new("Tess", None)]; + connection.insert_without_return(&users::table, &data).unwrap(); + + connection +}