This project is archived and is in readonly mode.

#2087 ✓wontfix
Tys von Gaza

Conditions on deep joins uses wrong table name

Reported by Tys von Gaza | February 26th, 2009 @ 06:51 PM

Example schema: AnswerSet has_many :answers

Answer belongs_to :answer_set has_many :answer_options

AnswerOptions belongs_to :answer

Simplified query needed: Need to find all AnswerOptions that belong to an AnswerSet that has an AnswerOption with a certain value...

The following will join the tables correctly AnswerOption.find(:all, :joins => { :answer => { :answer_set => { :answers => :answer_options }}})

The following will create the wrong condition names: AnswerOption.find(:all, :joins => { :answer => { :answer_set => { :answers => :answer_options }}}, :conditions => { :answer => { :answer_set => { :answers => { :answer_options => { :value => 1 } } }}})

ie returns: SELECT "answer_options".* FROM "answer_options" INNER JOIN "answers" ON "answers".id = "answer_options".answer_id INNER JOIN "answer_sets" ON "answer_sets".id = "answers".answer_set_id INNER JOIN "answers" answers_answer_sets ON answers_answer_sets.answer_set_id = answer_sets.id INNER JOIN "answer_options" answer_options_answers ON answer_options_answers.answer_id = answers_answer_sets.id WHERE ("answer_options"."value" = 1)

The desired query follow, notice the WHERE condition is ("answer_options_answers"."value" = 1) NOT ("answer_options"."value" = 1): SELECT "answer_options".* FROM "answer_options" INNER JOIN "answers" ON "answers".id = "answer_options".answer_id INNER JOIN "answer_sets" ON "answer_sets".id = "answers".answer_set_id INNER JOIN "answers" answers_answer_sets ON answers_answer_sets.answer_set_id = answer_sets.id INNER JOIN "answer_options" answer_options_answers ON answer_options_answers.answer_id = answers_answer_sets.id WHERE ("answer_options_answers"."value" = 1)

I'll see if I can create a test that fails after lunch.

Comments and changes to this ticket

  • Tys von Gaza

    Tys von Gaza February 26th, 2009 @ 07:12 PM

    The example can be simplified to a single belongs_to, has_many relationship:

    
    AnswerSet
      has_many :answers
    
    Answer
      belongs_to :answer_set
    

    Wrong query should look something like this: Answer.find(:all, :joins => { :answer_set => :answers }, :conditions => { :answer_set => { :answers => { :question_id => 1 } } })

    Generates: SELECT "answers".* FROM "answers" INNER JOIN "answer_sets" ON "answer_sets".id = "answers".answer_set_id INNER JOIN "answers" answers_answer_sets ON answers_answer_sets.answer_set_id = answer_sets.id WHERE ("answers"."question_id" = 1)

    Should generate: SELECT "answers".* FROM "answers" INNER JOIN "answer_sets" ON "answer_sets".id = "answers".answer_set_id INNER JOIN "answers" answers_answer_sets ON answers_answer_sets.answer_set_id = answer_sets.id WHERE ("answers_answer_sets"."question_id" = 1)

  • Tys von Gaza

    Tys von Gaza February 26th, 2009 @ 07:56 PM

    After thinking about this further, the root of the problem is how the conditions are built and doesn't have much to do with the joins.

    The following should really error out with bad sql, but it doesn't:

    Answer.find(:all, :conditions => { :answer_set => { :answers => { :question_id => 1 } } })

  • Tys von Gaza

    Tys von Gaza February 26th, 2009 @ 09:08 PM

    • Tag changed from 2.3-rc1, active_record, conditions, joins to 2.3-rc1, active_record, conditions, joins, tests

    Here is a patch that adds the tests, not really sure where to begin to fix the bug through.

  • CancelProfileIsBroken

    CancelProfileIsBroken August 5th, 2009 @ 02:12 PM

    • Tag changed from 2.3-rc1, active_record, conditions, joins, tests to 2.3-rc1, active_record, bugmash, conditions, joins, tests
  • Gabe da Silveira

    Gabe da Silveira August 8th, 2009 @ 09:41 PM

    verified that patch applies and tests fail on 2-3-stable

  • Gabe da Silveira

    Gabe da Silveira August 9th, 2009 @ 12:25 AM

    -1 on fixing this issue. After looking closely at this I don't think a fix is worth the overhead. In order to reliably fix this the conditions would need to know about joins and how the joined tables are named. That is add_joins! would need to probably pass the JoinDependency back to add_conditions! in AR::Base (or else duplicate the naming scheme which seems to be asking for trouble). Such a change would be a major refactoring that I think needs to be considered more holistically, and not considered before Rails 3 in any case. In order to merit the additional complexity this would need to solve a whole class of problems, or else inspire a refactoring that made the :conditions/:joins code more expressive.

    The workaround for this particular test case is simply to use:

    :conditions => { :'posts_authors.title' => "Welcome to the weblog" }
    

    instead of:

    :conditions => { :author => { :posts => { :title => "Welcome to the weblog" } } }
    
  • Steve St. Martin

    Steve St. Martin August 9th, 2009 @ 12:33 AM

    verified

    -1 I agree with Gabe on this one, the re-factoring it would take for this could cause more issues then it solves.

    At the point the developer is adding :join and :conditions they are already making an informed choice based off of their knowledge of the database schema, i do not feel manually specifying table names is that much added work
    (ideally these types of things should be pulled into the model anyway where it's more proper for it to 'know' about its relationships / naming conventions)

  • Michael Koziarski

    Michael Koziarski August 9th, 2009 @ 01:25 AM

    • State changed from “new” to “wontfix”

    agree with the earlier commenters.

    The Arel merge may make fixing this easier.

  • CancelProfileIsBroken

    CancelProfileIsBroken August 9th, 2009 @ 02:50 AM

    • Tag changed from 2.3-rc1, active_record, bugmash, conditions, joins, tests to 2.3-rc1, active_record, conditions, joins, tests
  • CancelProfileIsBroken

    CancelProfileIsBroken August 9th, 2009 @ 11:19 AM

    • Tag changed from 2.3-rc1, active_record, conditions, joins, tests to 2.3-rc1, active_record, conditions, joins, tests
    • Milestone cleared.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages