Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

find creates wrong SQL when you set "has_one :through" association on :join option

#3684

I ran into an issue on has_one :through.

First patch(test_join_has_many_throgh.diff) adds test cases to reproduce the issue.

When you define an association like the following ...

class Author
   has_many :author
end
class Post
   belongs_to :author
end
class Comment
   belongs_to :post
   has_one :post_author, :through=>:post, :source=>:author
end

And if you write the code like the following ...

comments = Comment.find :all, :conditions=>["authors.name = ?", "David"], :joins=>[:post_author]

It creates a SQL like this and you will get an exception.

SELECT "comments".* FROM "comments"
INNER JOIN "posts" ON ("comments"."id" = "posts"."post_id")    # <= Here is the problem
INNER JOIN "authors" ON ("authors"."id" = "posts"."author_id")  
WHERE (authors.name = 'David')

Since through_reflection is not :has_many but :belongs_to, the second line should be comments.post_id=posts.id.

The second patch(fix_join_has_one_through.diff) fixes this issue; it checks through_reflection.macro and creates a correct SQL in association.rb. It also includes test cases defined on the first patch and I checked the tests are passed as well as other existing tests.

(The patches are made against 2-3-stable.)

Reported by Tatsuya Ono · January 11th, 2010 @ 11:33 PM

State: duplicate
Milestone: none
Assigned to: Aaron Patterson Aaron Patterson
Importance: Low

Activity

  1. Tatsuya Ono
    Tatsuya Ono
    • Title changed from invalid SQL is created when you set "has_one :through" association to :join to find creates wrong SQL when you set "has_one :through" association on :join option

    January 11th, 2010 @ 11:35 PM

  2. Tatsuya Ono
    Tatsuya Ono
    • Tag changed from activerecord associations, activerecord to activerecord associations, 2.3.5, activerecord

    January 11th, 2010 @ 11:54 PM

  3. Paul Bowsher
    Paul Bowsher

    This patch will probably need to be forward-ported to ARel too

    January 12th, 2010 @ 08:06 AM

  4. Michael Koziarski
    Michael Koziarski
    • Assigned user set to Pratik

    January 16th, 2010 @ 02:34 AM

  5. Tatsuya Ono
    Tatsuya Ono
    • Tag changed from activerecord associations, 2.3.5, activerecord to 2.3.5, 2.3.6, activerecord, associations

    I upload a patch which does not contain unnecessary spaces.

    This patch will probably need to be forward-ported to ARel too

    I don't think that ARel itself is having problem related to this, though, I will make a patch against master. Thanks for the suggestion.

    February 4th, 2010 @ 01:21 AM

  6. Ryan Bigg
    Ryan Bigg

    Any further news on this Tatsuya?

    April 10th, 2010 @ 06:23 AM

  7. Ryan Bigg
    Ryan Bigg
    • Tag cleared.
    • Importance changed from to Low

    Automatic cleanup of spam.

    October 11th, 2010 @ 12:11 PM

  8. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 19th, 2010 @ 08:26 AM

  9. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:35 AM

  10. Jeff Kreeftmeijer
  11. Jon Leighton
    Jon Leighton
    • Assigned user changed from Pratik to Aaron Patterson

    This is a duplicate of #2801. I have run the tests proposed here against the fix I submitted to #2801 and verified that it does indeed fix this ticket too. So could this be closed please?

    December 19th, 2010 @ 05:05 PM

  12. Jon Leighton
    Jon Leighton
    • State changed from new to duplicate

    December 20th, 2010 @ 08:07 PM