Skip to content

Commit

Permalink
Replace left_outer_queriable! with a more generic blanket impl
Browse files Browse the repository at this point in the history
All of the queriable macros related to joins have caused issues, since
they are invalid via the orphan rules for coherence that exist today.
While I think we can relax those once specialization lands, or work
around it doing something ugly like this:
d65df55,
neither of those solutions are great right now.

This should remove the last blocker that makes this library impossible
to use in other crates. We still need to work around the trait coherence
rules for the `SelectableColumn` impls in `joinable!`, but that's a much
more livable workaround, as it will go away entirely once either
specialization or rust-lang/rfcs#1268 lands, as
I can replace that with a full blanket impl in the crate itself.

Once concern I do have is that we aren't doing much to enforce the
relationship between models. One thing that was already true, but is
coming to the forefront of my mind is that there's nothing to stop you
from doing `users.inner_join(posts).select((users::id, posts::title))`
and collecting that into `User { id: i32, name: String }`.

Do we even need to care though? Would caring be too tightly conflating
the concept of a record and a row from a specific table? Is this ever a
case that would be easy to accidentally get wrong? I don't so at this
time, you'd have to be pretty specific to get it wrong. I think this is
probably fine.

Once we add the work-around for the `SelectableColumn` stuff, we can
move the rest of the integration tests into the tests directory, and
start looking at how places like crates.io might use this library.
  • Loading branch information
sgrif committed Sep 30, 2015
1 parent 05499c2 commit 555993e
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 24 deletions.
19 changes: 0 additions & 19 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,22 +198,3 @@ macro_rules! joinable_inner {
}
}
}

macro_rules! left_outer_queriable {
($parent:ty, $parent_table:ident, $child:ty, $child_table:ident) => {
impl $crate::Queriable<($parent_table::SqlType, $crate::types::Nullable<$child_table::SqlType>)>
for ($parent, Option<$child>) {
type Row = (
<$parent as $crate::Queriable<$parent_table::SqlType>>::Row,
Option<<$child as $crate::Queriable<$child_table::SqlType>>::Row>,
);

fn build(row: Self::Row) -> Self {
(
<$parent as $crate::Queriable<$parent_table::SqlType>>::build(row.0),
row.1.map(<$child as $crate::Queriable<$child_table::SqlType>>::build),
)
}
}
}
}
1 change: 0 additions & 1 deletion src/tests/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ queriable! {
}

joinable!(posts -> users (user_id = id));
left_outer_queriable!(User, users, Post, posts);

#[derive(Debug, PartialEq, Eq)]
pub struct NewUser {
Expand Down
9 changes: 5 additions & 4 deletions src/types/impls/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ impl<T, ST> FromSql<Nullable<ST>> for Option<T> where

impl<T, ST> Queriable<Nullable<ST>> for Option<T> where
T: Queriable<ST>,
Option<T>: FromSqlRow<Nullable<ST>>,
Option<T::Row>: FromSqlRow<Nullable<ST>>,
ST: NativeSqlType,
{
type Row = Self;
fn build(row: Self) -> Self {
row
type Row = Option<T::Row>;

fn build(row: Self::Row) -> Self {
row.map(T::build)
}
}

Expand Down

0 comments on commit 555993e

Please sign in to comment.