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

Add UNION and UNION ALL support #2678

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@feymartynov
Contributor

feymartynov commented Sep 4, 2018

This PR adds support of UNION and UNION ALL:

customer_city_query = from c in Customer, select: c.city
from s in Supplier, select: s.city, union: customer_city_query
customer_city_query = Customer |> select([c], c.city)
Supplier |> select([s], s.city) |> union_all(customer_query)

Both can be used as keyword or expression versions. Multiple unions are also supported.

I made this because I had to optimize a complex query that had some OR conditions which made the optimizer unable to use indexes. This could be optimized by rewriting it with UNION ALL but Ecto didn't have such an ability. Rewriting the query as plain SQL was impossible because it was constructed of different subqueries spreaded all accross of my app's domain logic.

This is my first contribution here. Hope I got it correctly how Ecto.Query works under the hood, especially the Planner module. If anything is wrong please let me know and I'll update the PR.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Sep 4, 2018

Member
Member

josevalim commented Sep 4, 2018

Show outdated Hide outdated lib/ecto/query.ex Outdated
Show outdated Hide outdated lib/ecto/query.ex Outdated
@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Sep 5, 2018

Member

Thanks @feymartynov ! I really like the improvements and new tests you have added. I have added a last batch of comments and we should be good to go.

Member

josevalim commented Sep 5, 2018

Thanks @feymartynov ! I really like the improvements and new tests you have added. I have added a last batch of comments and we should be good to go.

@feymartynov

This comment has been minimized.

Show comment
Hide comment
@feymartynov

feymartynov Sep 10, 2018

Contributor

@josevalim I've added a couple of commits regarding your comments. Please check it out.

Contributor

feymartynov commented Sep 10, 2018

@josevalim I've added a couple of commits regarding your comments. Please check it out.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Sep 10, 2018

Member

Thank you @feymartynov! We are currently working on DBConnection 2.0 (which is a requirement for Ecto 3.0) and we should focus on Ecto again soon!

Member

josevalim commented Sep 10, 2018

Thank you @feymartynov! We are currently working on DBConnection 2.0 (which is a requirement for Ecto 3.0) and we should focus on Ecto again soon!

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Sep 26, 2018

Member

Merged manually, thank you! ❤️ 💚 💙 💛 💜

Member

josevalim commented Sep 26, 2018

Merged manually, thank you! ❤️ 💚 💙 💛 💜

@josevalim josevalim closed this Sep 26, 2018

@feymartynov feymartynov deleted the feymartynov:fey/union branch Sep 27, 2018

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