Skip to content
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

[Question] SubQueries capabilities #46

Open
danielnc opened this issue Feb 7, 2017 · 14 comments
Open

[Question] SubQueries capabilities #46

danielnc opened this issue Feb 7, 2017 · 14 comments

Comments

@danielnc
Copy link

danielnc commented Feb 7, 2017

Issue

Is it possible to use baby_squeel to create subqueries similar to the ones we have on Squeel?

I cannot find any documentation on this and we are trying to move from Squeel to Baby Squeel due to several bugs related to polymorphic relations and bind values on subqueries

Thanks

Reproduction

# Query 1
            notes_relation = Person.first.notes
            klazz_name = "Person"
            scope = Person.joins { notes_relation.as("notes").on { (~id == notes.notable_id) & (notes.notable_type == klazz_name) }.outer }.where { notes.note != nil }
# Query 2
subquery = OrderItem.group(:orderable_id).select { [orderable_id, sum(quantity * unit_price).as(amount)] }
Seat.joins { [payment.outer, subquery.as('seat_order_items').on { id == seat_order_items.orderable_id}.outer] }.
              select { [seat_order_items.amount, "seats.*"] }
@rzane
Copy link
Owner

rzane commented Feb 7, 2017

This is the same as #42. The exact expression you're trying to do isn't currently implemented, but baby_squeel does have limited support for subqueries.

  • Post.where.has { author.id.in Author.select(:id).limit(3) }
  • Post.where.has { exists Post.where(id: 1) }
  • Post.where.has { not_exists Post.where(id: 1) }

I'd be happy to accept a pull request.

@danielnc
Copy link
Author

danielnc commented Feb 7, 2017

@rzane glad to help but it's the first time I am taking a look at baby_squeel. Squeel is completely broken for our 4.2 upgrade so am I looking for options since we were doing pretty heavy usage of Squeel

Can you point me in the correct direction to try to understand how to start this PR? I am not familiar with this codebase and from my first look I was somewhat lost :(

I am assuming that given our heavy usage of Squeel I'll be able to contribute to baby_squeel to cover our usage cases

Thanks and hope you can guide me in the right direction

@rzane
Copy link
Owner

rzane commented Feb 7, 2017

I did a little investigation. Here's a gist of what I think it would take.

class Arel::Nodes::TableAlias
  def on(node)
    table = BabySqueel::Table.new(self)
    table.on(node)
  end
end

That'll get you part of the way there:

$ bin/console
> authors = Author.select(:id)
> puts Post.where.has { authors.as('a').on(author_id == author.id) }.to_sql
SELECT "posts".* FROM "posts" WHERE (INNER JOIN (SELECT "authors"."id" FROM "authors") a ON "posts"."author_id" = "authors"."id")

As you can see, it's not using the alias (a). The next step would be allowing #on to take a block, where inside the block, the aliased table name would resolve to the instance of BabySqueel::Table.

Notes:

  1. The signature for BabySqueel::Table#on should become def on(node = nil, &block). That way, you can choose if you want to give it a block or not.
  2. BabySqueel::DSL#evaluate will move up to BabySqueel::Table.
  3. A special case would be authors = Author.limit(3), because AR handles bind params in a different way for limit and offset. Gotta make sure there's tests for this.

@danielnc
Copy link
Author

danielnc commented Feb 7, 2017

But for your example node is Post

#<Arel::Table:0x007fea780b0210 @name="posts", @engine=Post(id: integer, title: string, author_id: integer, published_at: datetime, parent_id: integer, created_at: datetime, updated_at: datetime), @columns=nil, @aliases=[], @table_alias=nil, @primary_key=nil>

Its not related to Author so I don't have the alias to resolve

I am probably missing something that is in my face

@rzane
Copy link
Owner

rzane commented Feb 8, 2017

@danielnc I got this sort of working, but binds parameters don't work.

https://github.com/rzane/baby_squeel/compare/join-wip

@danielnc
Copy link
Author

danielnc commented Feb 8, 2017

@rzane thanks

I am investigating missing bind params and I believe its related to this: rails/arel#363

Still trying to learn more about AREL to see what needs to be done to use bind params correctly

@rzane
Copy link
Owner

rzane commented Feb 8, 2017

I think the bind values have to be copied from the relation you're joining to the parent relation. At one time, baby_squeel had to handle this sort of thing for normal joins 7ca9e32.

It's definitely not an issue with Active Record or with Arel. It's just that Arel is a private API within Rails, so relation.arel offers no guarantees.

@danielnc
Copy link
Author

danielnc commented Feb 8, 2017

Yes, problem is I don't have a ActiveRecord relation when calling DSL.evaluate
I only have access to the AST and I'm not 100% sure how can I get the list of bind_values easily without touching too much of Arel internals or traversing the entire tree

@rzane
Copy link
Owner

rzane commented Feb 8, 2017

Yes, that's true. Might have to implement a method on the DSL to handle the relation

authors = Author.limit(3)
Post.where.has { rel(authors).alias('a').on { ... } }

@danielnc
Copy link
Author

danielnc commented Feb 8, 2017

I got bind_values working following your suggestion
now need to make sure table and alias references works on where clauses
.on { a.id == author_id } should yield ON a."id" = "posts"."author_id"
and
.where.has { a.name == "AuthorName" } should yield "a"."name" = "AuthorName"

Any advice on how to properly handle where.has a.name?
for the first one I'll have to follow something similar to squeel and add a ~ operator to reference top level tables

@rzane
Copy link
Owner

rzane commented Feb 9, 2017

Yeah, you're right about ~, and this is the key to handling the alias name:

if _table.kind_of?(Arel::Nodes::TableAlias) && _table.name == name.to_s
self

@daliborfilus
Copy link

I am migrating from sqeel to baby_squeel (thanks for all the work!) and I came across a little issue during migration. I have master scope which selects from some huge table and then I do some filtering on it. I have a situation (multiple times in this project) where I need to reference the master table from the subquery and it throws undefined column exception.

Something like this:

s = MasterList.where(...)
subquery = Address.where.has { address_id == master_list.address_id }
s.where { exists subquery }

master_list is undefined in this case. It is defined in the final query though. I cannot find in documentation/wiki how exactly this translates to baby_sqeel, if it's possible at all. Any hints would be appreciated :-)

@rzane
Copy link
Owner

rzane commented Mar 30, 2018

Hey @noice, could you create a new issue? This one is tracking a pretty specific feature request. Plus, if you could fill out the issue template when you create the issue, that would be especially appreciated. I don't know what your schema looks like or how your models are associated, so without that information, it'll really, really difficult to help you.

@daliborfilus
Copy link

That was a quick response! :-) Of course, I've created minimalistic example and created new issue #88 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants