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.

[PATCH] sanitize_sql_hash_for_conditions uses incorrect table_name

#2462

If you add the following test to activerecord/test/cases/finder_test.rb


  def test_find_with_include_and_conditions_with_dot
    Post.find(:all,
              :include => [ :authors ],
              :conditions => {
                :title => 'FOO',
                'authors.name' => 'BAR'
               }
             )
  end

It will blow up with the error:


ActiveRecord::StatementInvalid: Mysql::Error: Unknown column 'authors.title' in 'where clause': SELECT `posts`.`id` AS t0_r0, `posts`.`author_id` AS t0_r1, `posts`.`title` AS t0_r2, `posts`.`body` AS t0_r3, `posts`.`type` AS t0_r4, `posts`.`comments_count` AS t0_r5, `posts`.`taggings_count` AS t0_r6, `authors`.`id` AS t1_r0, `authors`.`name` AS t1_r1, `authors`.`author_address_id` AS t1_r2, `authors`.`author_address_extra_id` AS t1_r3 FROM `posts`  LEFT OUTER JOIN `categorizations` ON (`posts`.`id` = `categorizations`.`category_id`)  LEFT OUTER JOIN `authors` ON (`authors`.`id` = `categorizations`.`author_id`) WHERE (`authors`.`name` = 'BAR' AND `authors`.`title` = 'FOO') 

I traced this back to the method: activerecord/lib/active_record/base.rb#sanitize_sql_hash_for_conditions

If a condition defines a column and includes a '.', the table name will be used for any subsequent columns that do not contain a '.' in them when it should be using quoted_table_name.

Reported by ekolve · April 8th, 2009 @ 09:33 PM

State: stale
Milestone: 3.x
Assigned to: Pratik Pratik
Importance: none

Activity

  1. ekolve
    ekolve

    This occurs for version 2.3.2

    April 8th, 2009 @ 09:34 PM

  2. Will Bryant
    Will Bryant

    Use :conditions => {:authors => {:name => 'BAR'}}. AFAIK :conditions => {'authors.name' => 'BAR'} has never been officially supported, though it may have worked previously.

    April 9th, 2009 @ 03:27 AM

  3. Joel Chippindale
    Joel Chippindale
    • Tag set to activerecord, patch

    The dot notation continues to work and is included in the finder unit tests on master.

    The attached fix resolves this problem with the table_names from dot notation keys leaking into subsequent conditions.

    June 16th, 2009 @ 10:03 AM

  4. Joel Chippindale
    Joel Chippindale
    • Title changed from sanitize_sql_hash_for_conditions uses incorrect table_name to [PATCH] sanitize_sql_hash_for_conditions uses incorrect table_name

    June 17th, 2009 @ 11:13 AM

  5. Joel Chippindale
    Joel Chippindale

    Judging by responses I have had to this ticket this does not seem to be a well known feature however it has been around for a while and is clearly included in the rails guides

    June 20th, 2009 @ 05:50 AM

  6. Pratik
    Pratik
    • State changed from new to incomplete

    Hey,

    Your patch is missing tests. Could you please add a test ?

    Thanks.

    June 24th, 2009 @ 09:30 AM

  7. Joel Chippindale
    Joel Chippindale
    • Assigned user set to Pratik

    Here's a new version with a test.

    June 26th, 2009 @ 12:22 PM

  8. Pratik
    Pratik

    Hey Joel,

    Is it possible to write the test without stubbing ? Also the test should be using :joins instead of :include - as per the guide and the recommended way.

    Thanks.

    July 29th, 2009 @ 07:17 PM

  9. Joel Chippindale
    Joel Chippindale

    New patch attached using :joins instead of :include and without the stubbing.

    Be aware that (since removing the stubbing) the existing code will pass this test if your installation of ruby happens to process the 'title' hash key before the 'authors.name' key. The patched code will pass the test independently of the order in which the hash keys are processed.

    August 4th, 2009 @ 01:00 PM

  10. Joel Chippindale
  11. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  12. Santiago Pastorino
    Santiago Pastorino
    • State changed from incomplete to open
    • Importance changed from to

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 2nd, 2011 @ 05:03 PM

  13. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 05:03 PM