collection_singular_ids= setter broken when keys are strings on postgresql [Rails 3.0.8.rc1] #1326

Closed
paneq opened this Issue May 26, 2011 · 10 comments

Comments

Projects
None yet
3 participants
@paneq
Contributor

paneq commented May 26, 2011

String column values are escaped improperly in SQL queries.

class Function < ActiveRecord::Base {
    :short_name => :string,
          :path => :string,
        :hidden => :boolean,
    :created_at => :datetime,
    :updated_at => :datetime
}

class Role < ActiveRecord::Base {
    :short_name => :string,
        :system => :boolean,
    :created_at => :datetime,
    :updated_at => :datetime
}
f = Function.first

#<Function:0xd40d8d4> {
    :short_name => "ManagePermissions",
          :path => "permissions",
        :hidden => false,
    :created_at => Mon, 23 May 2011 08:43:04 UTC +00:00,
    :updated_at => Mon, 23 May 2011 08:43:04 UTC +00:00
}
ruby-1.9.2-p136 :013 > f.role_ids
[
    [0] "Administrator"
]
ruby-1.9.2-p136 :014 > f.role_ids= f.role_ids
ActiveRecord::StatementInvalid: PGError: ERROR:  column "managepermissions" does not exist
LINE 1: ...s".role_id WHERE (("function_roles".function_id = ManagePerm...
                                                             ^
: SELECT "roles".* FROM "roles" INNER JOIN "function_roles" ON "roles".short_name = "function_roles".role_id WHERE (("function_roles".function_id = ManagePermissions))
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:207:in `rescue in log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:199:in `log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:514:in `execute'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:1004:in `select_raw'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:997:in `select'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/database_statements.rb:7:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/query_cache.rb:56:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:473:in `find_by_sql'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation.rb:64:in `to_a'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:143:in `all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:105:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:444:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `block in find_target'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:1127:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_proxy.rb:207:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `find_target'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_collection.rb:410:in `load_target'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_collection.rb:356:in `replace'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations.rb:1515:in `block in collection_accessor_methods'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations.rb:1523:in `block in collection_accessor_methods'
    from (irb):14
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:44:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:8:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands.rb:23:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'ruby-1.9.2-p136 :015 > 
@paneq

This comment has been minimized.

Show comment Hide comment
@paneq

paneq May 26, 2011

Contributor

Seems to be deeper problem with associations:

class Person < BusinessObject
  has_many :contractings
  has_many :contracts, :through => :contractings
end

class Person < BusinessObject {
             :id => :string,
           :type => :string,
}

class Contract < BusinessObject {
             :id => :string,
           :type => :string
}

class Contracting < Foundation {
    :contract_id => :string,
      :person_id => :string
}


Person.first.contracts

Exception:

ActiveRecord::StatementInvalid: PGError: ERROR:  operator does not exist: character varying = integer
LINE 1: ...type" = 'Contract' AND (("contractings".person_id = 00000002...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
: SELECT "business_objects".* FROM "business_objects" INNER JOIN "contractings" ON "business_objects".id = "contractings".contract_id WHERE "business_objects"."type" = 'Contract' AND (("contractings".person_id = 00000002))
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:207:in `rescue in log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:199:in `log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:514:in `execute'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:1004:in `select_raw'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:997:in `select'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/database_statements.rb:7:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/query_cache.rb:56:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:473:in `find_by_sql'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation.rb:64:in `to_a'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:143:in `all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:105:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:444:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `block in find_target'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:1127:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_proxy.rb:207:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `find_target'
... 1 levels...
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_proxy.rb:213:in `method_missing'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_collection.rb:433:in `method_missing'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/will_paginate-2.3.15/lib/will_paginate/finder.rb:170:in `method_missing_with_paginate'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:180:in `declassify'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:174:in `printable'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/mixin/active_record.rb:16:in `printable_with_active_record'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/mixin/active_support.rb:16:in `printable_with_active_support'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:149:in `awesome'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/core_ext/kernel.rb:10:in `ai'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/core_ext/kernel.rb:15:in `ap'
    from /home/rupert/.irbrc:15:in `output_value'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:44:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:8:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands.rb:23:in `<top (required)>'
Contributor

paneq commented May 26, 2011

Seems to be deeper problem with associations:

class Person < BusinessObject
  has_many :contractings
  has_many :contracts, :through => :contractings
end

class Person < BusinessObject {
             :id => :string,
           :type => :string,
}

class Contract < BusinessObject {
             :id => :string,
           :type => :string
}

class Contracting < Foundation {
    :contract_id => :string,
      :person_id => :string
}


Person.first.contracts

Exception:

ActiveRecord::StatementInvalid: PGError: ERROR:  operator does not exist: character varying = integer
LINE 1: ...type" = 'Contract' AND (("contractings".person_id = 00000002...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
: SELECT "business_objects".* FROM "business_objects" INNER JOIN "contractings" ON "business_objects".id = "contractings".contract_id WHERE "business_objects"."type" = 'Contract' AND (("contractings".person_id = 00000002))
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:207:in `rescue in log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:199:in `log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:514:in `execute'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:1004:in `select_raw'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/postgresql_adapter.rb:997:in `select'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/database_statements.rb:7:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/connection_adapters/abstract/query_cache.rb:56:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:473:in `find_by_sql'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation.rb:64:in `to_a'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:143:in `all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/relation/finder_methods.rb:105:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:444:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `block in find_target'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/base.rb:1127:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_proxy.rb:207:in `with_scope'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/has_many_through_association.rb:84:in `find_target'
... 1 levels...
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_proxy.rb:213:in `method_missing'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/activerecord-3.0.8.rc1/lib/active_record/associations/association_collection.rb:433:in `method_missing'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/will_paginate-2.3.15/lib/will_paginate/finder.rb:170:in `method_missing_with_paginate'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:180:in `declassify'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:174:in `printable'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/mixin/active_record.rb:16:in `printable_with_active_record'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/mixin/active_support.rb:16:in `printable_with_active_support'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/awesome_print.rb:149:in `awesome'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/core_ext/kernel.rb:10:in `ai'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/awesome_print-0.2.1/lib/ap/core_ext/kernel.rb:15:in `ap'
    from /home/rupert/.irbrc:15:in `output_value'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:44:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands/console.rb:8:in `start'
    from /home/rupert/.rvm/gems/ruby-1.9.2-p136/gems/railties-3.0.8.rc1/lib/rails/commands.rb:23:in `<top (required)>'
@jonleighton

This comment has been minimized.

Show comment Hide comment
@jonleighton

jonleighton May 26, 2011

Member

Is this a regression from 3.0.7?

Member

jonleighton commented May 26, 2011

Is this a regression from 3.0.7?

@paneq

This comment has been minimized.

Show comment Hide comment
@paneq

paneq May 26, 2011

Contributor

Yes. It works fine on 3.0.7 .

Contributor

paneq commented May 26, 2011

Yes. It works fine on 3.0.7 .

@ghost ghost assigned tenderlove May 26, 2011

@tenderlove

This comment has been minimized.

Show comment Hide comment
@tenderlove

tenderlove May 26, 2011

Member

@paneq would you mind providing the important parts of the schema.rb along with actual ruby for constructing the objects? Then I can get this fixed.

Member

tenderlove commented May 26, 2011

@paneq would you mind providing the important parts of the schema.rb along with actual ruby for constructing the objects? Then I can get this fixed.

@paneq

This comment has been minimized.

Show comment Hide comment
@paneq

paneq May 26, 2011

Contributor

Stolen tables from existing rails tests schema so it is easier to write the test :-)

 create_table :countries, :force => true, :id => false, :primary_key => 'country_id' do |t|
   t.string :country_id
   t.string :name
 end

create_table :treaties, :force => true, :id => false, :primary_key => 'treaty_id' do |t|
   t.string :treaty_id
   t.string :name
 end

create_table :countries_treaties, :force => true, :id => false do |t|
  t.string :country_id, :null => false
  t.string :treaty_id, :null => false
  t.datetime :created_at
  t.datetime :updated_at
end

Models differ a little from current models in rails tests:

class Country < ActiveRecord::Base
  set_primary_key :country_id
  has_many :country_treaties
  has_many :treaties, :through => :country_treaties
end

class CountryTreaty < ActiveRecord::Base
  set_table_name :countries_treaties
  belongs_to :country
  belongs_to :treaty
end

class Treaty < ActiveRecord::Base
  set_primary_key :treaty_id
  has_many :country_treaties
  has_many :countries, :through => :country_treaties
end

Things that lead to an exception:

Country.create! do |c|
 c.id = "PL"
end
Treaty.create! do |t|
 t.id = "UE"
end

CountryTreaty.create!(:country_id => "PL", :treaty_id => "UE")

c = Country.first 
c.treaties # exception
c.treaty_ids = c.treaty_ids # exception

I hope this helps and makes working on this little bug nice and quick.

Contributor

paneq commented May 26, 2011

Stolen tables from existing rails tests schema so it is easier to write the test :-)

 create_table :countries, :force => true, :id => false, :primary_key => 'country_id' do |t|
   t.string :country_id
   t.string :name
 end

create_table :treaties, :force => true, :id => false, :primary_key => 'treaty_id' do |t|
   t.string :treaty_id
   t.string :name
 end

create_table :countries_treaties, :force => true, :id => false do |t|
  t.string :country_id, :null => false
  t.string :treaty_id, :null => false
  t.datetime :created_at
  t.datetime :updated_at
end

Models differ a little from current models in rails tests:

class Country < ActiveRecord::Base
  set_primary_key :country_id
  has_many :country_treaties
  has_many :treaties, :through => :country_treaties
end

class CountryTreaty < ActiveRecord::Base
  set_table_name :countries_treaties
  belongs_to :country
  belongs_to :treaty
end

class Treaty < ActiveRecord::Base
  set_primary_key :treaty_id
  has_many :country_treaties
  has_many :countries, :through => :country_treaties
end

Things that lead to an exception:

Country.create! do |c|
 c.id = "PL"
end
Treaty.create! do |t|
 t.id = "UE"
end

CountryTreaty.create!(:country_id => "PL", :treaty_id => "UE")

c = Country.first 
c.treaties # exception
c.treaty_ids = c.treaty_ids # exception

I hope this helps and makes working on this little bug nice and quick.

@tenderlove

This comment has been minimized.

Show comment Hide comment
@tenderlove

tenderlove May 26, 2011

Member

@paneq awesome! Thank you! I really appreciate the detailed report!

Member

tenderlove commented May 26, 2011

@paneq awesome! Thank you! I really appreciate the detailed report!

@paneq

This comment has been minimized.

Show comment Hide comment
@paneq

paneq May 26, 2011

Contributor

Always pleasure to work with you. I totally like the "veto" idea for Rails releases.

Contributor

paneq commented May 26, 2011

Always pleasure to work with you. I totally like the "veto" idea for Rails releases.

@tenderlove

This comment has been minimized.

Show comment Hide comment
@tenderlove

tenderlove May 26, 2011

Member

Just to update: I've reproduced the bug. Looking in to a fix!

Member

tenderlove commented May 26, 2011

Just to update: I've reproduced the bug. Looking in to a fix!

@paneq

This comment has been minimized.

Show comment Hide comment
@paneq

paneq May 26, 2011

Contributor

Good luck. It's getting midnight at my time zone so I'm going to sleep.

Contributor

paneq commented May 26, 2011

Good luck. It's getting midnight at my time zone so I'm going to sleep.

@tenderlove

This comment has been minimized.

Show comment Hide comment
@tenderlove

tenderlove May 26, 2011

Member

I've found the commit that caused the regression: 9f5ab9a

I fixed this, but accidentally put the wrong ticket number in the commit. It is fixed with 179a8a4.

Member

tenderlove commented May 26, 2011

I've found the commit that caused the regression: 9f5ab9a

I fixed this, but accidentally put the wrong ticket number in the commit. It is fixed with 179a8a4.

@tenderlove tenderlove closed this May 26, 2011

jake3030 pushed a commit to jake3030/rails that referenced this issue Jun 28, 2011

Require active_support/secure_random for Ruby 1.9.
[#1326 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment