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.

Add magic encoding comment to generated files

#2949

I just upgraded from Rails 2.2.2 to 2.3.3 and now ActiveRecord generates invalid SQL for named_scopes with :conditions + :joins.

Example extracted from my current project:

class ProviderIdMap < PCM::Model
  belongs_to :provider_id_map_category

  named_scope :defaults, :conditions => { :use_as_default => true }
  named_scope :tags,
              :joins      => [ :provider_id_map_category ],
              :conditions =>
              { "provider_id_map_category.provider_id_map_category_ud" => "TAG" }

  # ...
end

Doing:

ProviderIdMap.defaults.tags.count

in 2.2.2 generates the following SQL:

SELECT count(*) AS count_all 
FROM [provider_id_map] 
INNER JOIN [provider_id_map_category] 
      ON [provider_id_map_category].provider_id_map_category_id = 
         [provider_id_map].provider_id_map_category_id 

WHERE (([provider_id_map_category].[provider_id_map_category_ud] = 'TAG') 
  AND ([provider_id_map].[use_as_default] = 1))

Watch that last line - In 2.3.3 this is generated:

SELECT count(*) AS count_all 
FROM [provider_id_map]   
INNER JOIN [provider_id_map_category] 
      ON [provider_id_map_category].provider_id_map_category_id =
         [provider_id_map].provider_id_map_category_id  

WHERE (([provider_id_map_category].[provider_id_map_category_ud] = 'TAG'
  AND [provider_id_map_category].[use_as_default] = 1))

Yikes :( The use_as_default condition is mapped to the wrong table. It should go with provider_id_map, but instead in 2.3.3 it suddenly is attached to provider_id_map_category -> SQL Error:

  ActiveRecord::StatementInvalid: DBI::DatabaseError: S0022 (207) Invalid column name 'use_as_default'.

Reported by Casper · July 24th, 2009 @ 04:54 PM

State: resolved
Milestone: none
Assigned to: Jeremy Kemper Jeremy Kemper
Importance: Low

Activity

  1. Casper
    Casper

    Here's a simplified model and migration. Making the problem easily reproducible:

    class Widget < ActiveRecord::Base
       belongs_to :window


    named_scope :enabled, :conditions => { :enabled => true } named_scope :desktop,

               :joins =&gt; :window,
               :conditions =&gt; { &quot;windows.name&quot; =&gt; &quot;desktop&quot; }
    
    
    
    
    end

    Migration to use for testing:

         create_table :widgets do |t|

       t.belongs_to :window
    
       t.string :name
       t.boolean :enabled
     end
    
     create_table :windows do |t|
       t.string :name
     end</code>
    
    
    
    

    Tested on edge rails (3.0pre), 2.3.3, 2.3.2, 2.3.1, all fail. Works on 2.3.0 and earlier versions.

    What is even stranger is that with this minor modification it works on 2.3.3 also:

    class Widget < ActiveRecord::Base
       belongs_to :window
    
       named_scope :enabled, :conditions => { :enabled => true }
       named_scope :desktop,
                   :joins => :window,
                   :conditions => [ "windows.name = ?", "desktop" ]
    end
    

    I haven't been able to figure out what changed between 2.3.0 and 2.3.1 to introduce the bug. Git diff didn't give any good clues, nor did the log entries. Really strange problem.

    July 25th, 2009 @ 08:38 PM

  2. Michael Koziarski
    Michael Koziarski
    • Tag changed from 2.3.3, activerecord, conditions, joins, named_scope, sql to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql

    August 3rd, 2009 @ 06:05 AM

  3. Arthur Zapparoli
    Arthur Zapparoli

    Verified this generates invalid SQL on 2-3-stable

    Besides working with conditions Array, it also works with:

      class Widget < ActiveRecord::Base
        belongs_to :window
    
        named_scope :enabled, :conditions => { :enabled => true }
        named_scope :desktop,
                    :joins => :window,
                    :conditions => { :windows => { :name => "desktop" } }
      end
    

    August 9th, 2009 @ 05:33 AM

  4. Arthur Zapparoli
    Arthur Zapparoli

    Ok, this is really weird. I think it is related directly to the model name (or only the first letter). I was trying to create a failing test patch, but, after playing with it again, I couldn't reproduce with this:

    class Comment < ActiveRecord::Base
      belongs_to :post
    
      named_scope :enabled, :conditions => { :enabled => true }
      named_scope :desktop,
                  :joins => :post,
                  :conditions => { "posts.name" => "desktop" }
    end
    
    class Post < ActiveRecord::Base
    end
    

    Calling Comment.enabled.desktop worked fine. Then, I tried with:

    class Comment < ActiveRecord::Base
      belongs_to :cost
    
      named_scope :enabled, :conditions => { :enabled => true }
      named_scope :desktop,
                  :joins => :cost,
                  :conditions => { "costs.name" => "desktop" }
    end
    
    class Cost < ActiveRecord::Base
    end
    

    Then, calling Comment.enabled.desktop brings the SQL error.

    I can't add a failing test patch without adding new models to the test suite. How should I proceed in this case?

    Here's full models:

    class Comment < ActiveRecord::Base
      belongs_to :cost
      belongs_to :post
    
      named_scope :enabled, :conditions => { :enabled => true }
      named_scope :cost_desktop,
                  :joins => :cost,
                  :conditions => { "costs.name" => "desktop" }
      
      named_scope :post_desktop,
                  :joins => :post,
                  :conditions => { "posts.name" => "desktop" }
    end
    
    class Cost < ActiveRecord::Base
    end
    
    class Post < ActiveRecord::Base
    end
    

    Here's schema:

    ActiveRecord::Schema.define do
      create_table :comments, :force => true do |t|
        t.integer :post_id
    
        t.string :name
        t.boolean :enabled
      end
    
      create_table :costs, :force => true do |t|
        t.string :name
      end
      
      create_table :posts, :force => true do |t|
        t.string :name
      end
    end
    

    Calling Comment.enabled.post_desktop works, but Comment.enabled.cost_desktop doesn't.

    August 9th, 2009 @ 05:40 PM

  5. Arthur Zapparoli
    Arthur Zapparoli

    Here's the SQL generated for Comment.enabled.post_desktop

    SELECT `comments`.* FROM `comments` INNER JOIN `posts` ON `posts`.id = `comments`.post_id WHERE ((`comments`.`enabled` = 1 AND `posts`.`name` = 'desktop'))
    

    And, here's the one generated for Comment.enabled.cost_desktop

    SELECT `comments`.* FROM `comments` INNER JOIN `costs` ON `costs`.id = `comments`.cost_id WHERE ((`costs`.`name` = 'desktop' AND `costs`.`enabled` = 1))
    

    Which gives me:

    Mysql::Error: Unknown column 'costs.enabled' in 'where clause'
    

    August 9th, 2009 @ 05:50 PM

  6. Tristan Dunn
    Tristan Dunn
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified

    The issue is with sanitize_sql_hash_for_conditions overwriting the table name if one is present in the conditions. Solution was to simply reset it at the beginning of each iteration.

    Patches are attached for master and 2-3-stable.

    August 9th, 2009 @ 08:21 PM

  7. Casper
    Casper
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql

    Ok since you found the weirdness with the first letter in the class name I'll tell you something even more strange.

    I didn't post it earlier since I thought perhaps the problem was between my ears.

    I hunted down the git change commit that causes the error. Since I found the problem appeared between 2.3.0 and 2.3.1 with some trial and error I found two commits close to each other -> with one it works, with the other not.

    To reproduce:

    1. Clone rails from github into vendor of your test project
    2. The commit that works is beca1f2, check it out (git checkout beca1f2)
    3. Tag the commit for convenience (git tag works)
    4. The commit that fails is 2ecc678, check it out (git checkout 2ecc678)
    5. Tag it for convenience (git tag broken)

    Now you can switch back and forth (git checkout works / git checkout broken) and test it.

    Next..look at the diff between the two commits:
    git diff works..broken

    Then scratch your head really hard..on my machine the only relevant change according to git is in action_controller/rescue.rb. How this can affect the associations for named_scope I have no clue, but it does.

    August 9th, 2009 @ 08:25 PM

  8. Casper
    Casper
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified

    Tristan updated ticket at same time. Verified his patch works on my machine. Reapplied his tags. Disregard my previous post :-/

    Thanks for the fix.

    August 9th, 2009 @ 08:35 PM

  9. Derander
    Derander
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql

    +1 verified.

    Both patches apply cleanly.

    August 9th, 2009 @ 08:36 PM

  10. Arthur Zapparoli
    Arthur Zapparoli
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql to 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified

    +1 for Tristan's patch. Applies cleanly to 2-3-stable and master.

    Nice catch! :)

    August 9th, 2009 @ 08:41 PM

  11. Repository
  12. Repository
  13. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from 2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql, verified to 2.3.3, activerecord, conditions, joins, named_scope, sql, verified
    • Milestone cleared.

    August 10th, 2009 @ 02:15 AM

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

    Automatic cleanup of spam.

    October 9th, 2010 @ 10:13 PM

  15. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:38 AM

  16. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:40 AM

  17. Jeff Kreeftmeijer
  18. Jeff Kreeftmeijer
  19. Jeff Kreeftmeijer
  20. bingbing