#2949 Add magic encoding comment to generated files - Ruby on Rails

You do not have access to this ticket

This project is archived and is in readonly mode.

#2949 ✓resolved

Add magic encoding comment to generated files

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

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'.

Comments and changes to this ticket

Casper July 25th, 2009 @ 08:38 PM

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.

Michael Koziarski August 3rd, 2009 @ 06:05 AM
- Tag changed from “2.3.3, activerecord, conditions, joins, named_scope, sql” to “2.3.3, activerecord, bugmash, conditions, joins, named_scope, sql”

Arthur Zapparoli August 9th, 2009 @ 05:33 AM

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

Arthur Zapparoli August 9th, 2009 @ 05:40 PM

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.

Arthur Zapparoli August 9th, 2009 @ 05:50 PM

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'

You flagged this item as spam.
Tristan Dunn August 9th, 2009 @ 08:21 PM
- 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.
- named_scope_table_names_2-3-stable.patch 2.5 KB
- named_scope_table_names_master.patch 2.5 KB
You flagged this item as spam.
Casper August 9th, 2009 @ 08:25 PM
- 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.
You flagged this item as spam.
Casper August 9th, 2009 @ 08:35 PM
- 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.
You flagged this item as spam.
Derander August 9th, 2009 @ 08:36 PM
- 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.
You flagged this item as spam.
Arthur Zapparoli August 9th, 2009 @ 08:41 PM
- 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! :)
Repository August 10th, 2009 @ 12:44 AM
- State changed from “new” to “resolved”
(from [491f1b5f36a11427decc5d7f3558b5c3f5f243c1]) Prevent overwriting of table name in merging SQL conditions [#2949 state:resolved] http://github.com/rails/rails/commit/491f1b5f36a11427decc5d7f3558b5...
Repository August 10th, 2009 @ 12:44 AM
(from [0472839d684d836e1e7ecd39722c313410d76d5b]) Prevent overwriting of table name in merging SQL conditions [#2949 state:resolved] http://github.com/rails/rails/commit/0472839d684d836e1e7ecd39722c31...
CancelProfileIsBroken August 10th, 2009 @ 02:15 AM
- 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.
Ryan Bigg October 9th, 2010 @ 10:13 PM
- Tag cleared.
- Importance changed from “” to “Low”
Automatic cleanup of spam.
Ryan Bigg October 21st, 2010 @ 03:38 AM
Automatic cleanup of spam.
Ryan Bigg October 21st, 2010 @ 03:40 AM
Automatic cleanup of spam.
Jeff Kreeftmeijer October 24th, 2010 @ 01:10 PM
Automatic cleanup of spam.
Jeff Kreeftmeijer October 24th, 2010 @ 01:13 PM
Automatic cleanup of spam.
Jeff Kreeftmeijer October 24th, 2010 @ 01:14 PM
Automatic cleanup of spam.
You flagged this item as spam.
bingbing March 26th, 2011 @ 01:19 AM
gucci watches