New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support union / intersect / except #33

Open
Meyermagic opened this Issue Nov 29, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@Meyermagic

Meyermagic commented Nov 29, 2015

http://www.postgresql.org/docs/current/static/queries-union.html

These are quite useful for set-like queries.

@sgrif

This comment has been minimized.

Member

sgrif commented Nov 29, 2015

Definitely reasonable to support. Not going to assign it a milestone just yet, but if anyone wants to tackle it, feel free. Shouldn't be hard to implement, just need to make sure that union, intersect, and except can be called multiple times, but the other query DSL methods cannot be called after one of these methods.

@sgrif sgrif added the enhancement label Nov 29, 2015

@Meyermagic

This comment has been minimized.

Meyermagic commented Apr 19, 2016

I'm going to start working on this soon (those livestreams are good advertising). I'm not familiar with your codebase, though, and want to make sure I have the right idea of how this will work.

I haven't yet started coding, but I see two main issues right now:

  1. Union/Intersect/Except only work with SELECT in at least PG 9.4, and not with UPDATE ... RETURNING even with a compatible type. So I can't just use AsQuery as is while relying on matching SqlType.
  2. Union/Intersect/Except can be called on a simple select query or a compound one, but most DSL methods can only be called on simple queries.

My high-level plan is then:

Introduce a CompoundSelectStatement enum with Union, Intersect, and Except variants. This should satisfy AsQuery when T: AsQuery, U: AsQuery with compatible types, but with an additional bound to satisfy 1.

Introduce three new DSL methods UnionDsl, IntersectDsl, and ExceptDsl which also require the trait added to satisfy 1.

Add a new bound to the other DSL methods, something like NotCompoundQuery, to satisfy 2.

Is there a Contribution Guide or architecture document or similar? I feel like a higher-level perspective might be helpful.

Disclaimer: I haven't put down any code for this yet, only poking around on GitHub. This might be way off-base.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 19, 2016

There's some random scattered thoughts with regards to contributing here. #120

I'd avoid having the types returned by these methods implement any of the DSLs, really, and instead require a fully built query. We want to avoid things like enums if possible, as they carry runtime information that can't always be inlined, and can't be reasoned about at the type level.

Go ahead and rely on AsQuery, we can add marker traits if needed to filter out the problem cases.

Keep in mind that anything adding support for something like this would need to be thoroughly tested.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 21, 2016

Gave this a little thought. It actually should be extremely straightforward, just needs a lot of tests.

trait UnionDsl<Other: AsQuery<SqlType=Self::SqlType>>: AsQuery {
    type Output = Query<SqlType=Self::SqlType>;

    fn union(self, other: Other) -> Self::Output;
}

impl<T, U> UnionDsl<U> for T where
    T: AsQuery,
    U: AsQuery<SqlType=T::SqlType>,
{
    type Output = UnionQuery<T::Query, U::Query>;

    fn union(self, other: U) -> Self::Output {
        UnionQuery::new(self.as_query(), other.as_query())
    }
}

pub struct UnionQuery<T, U> {
    left: T,
    right: U,
}

impl<T, U, DB> QueryFragment<DB> for UnionQuery<T, U> where
    // you know the drill
{
    // impls...
}

impl<T, U> Query for UnionQuery<T, U> where
    T: Query,
    U: Query<SqlType=T::SqlType>,
{
    type SqlType = T::SqlType;
}

Note: We may want to drop the constraints on UnionDsl and instead just have them on the impl of Query for the resulting UnionQuery, as it can actually be frustratingly difficult to satisfy that those constraints hold.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 21, 2016

And while I'd have UnionQuery and friends implement UnionDsl, IntersectDsl and ExceptDsl, they shouldn't implement any others. They need to take fully constructed queries and can't be modified afterwards. Also, for ease of review, please submit each of those as a separate PR.

@robertmaloney

This comment has been minimized.

Contributor

robertmaloney commented Apr 30, 2016

It seems that both SQLite and Pg support ORDER BY, LIMIT, OFFSET, and maybe others without treating it as a subquery. Should we implement those DSLs?

@Meyermagic

This comment has been minimized.

Meyermagic commented Apr 30, 2016

Yes, I noticed that too. Should have a patch by Monday if I have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment