This project is archived and is in readonly mode.

#5125 ✓resolved
Robert Pankowecki

collection_singular_ids= method raises an exception when primary_key is string (Rails 3)

Reported by Robert Pankowecki | July 15th, 2010 @ 05:53 PM | in 3.x

Using collection_singular_ids= (method generated by has_many) raises an exception when class primary_key is string.

Example:

class Role < ActiveRecord::Base
  has_many :function_roles
  has_many :roles, :through => :function_roles
end

class Function < ActiveRecord::Base
  set_primary_key :short_name # this is string
end

Role.first.function_ids = ['ManagePermissions']

ActiveRecord::StatementInvalid: PGError: ERROR:  operator does not exist: character varying = integer
LINE 1: ...  "functions" WHERE     ("functions"."short_name" = 0) LIMIT...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
: SELECT     "functions".* FROM       "functions" WHERE     ("functions"."short_name" = 0) LIMIT 1
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/abstract_adapter.rb:210:in `rescue in log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/abstract_adapter.rb:200:in `log'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/postgresql_adapter.rb:464:in `execute'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/postgresql_adapter.rb:954:in `select_raw'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/postgresql_adapter.rb:941:in `select'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/abstract/database_statements.rb:7:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/connection_adapters/abstract/query_cache.rb:56:in `select_all'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/base.rb:431:in `find_by_sql'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation.rb:64:in `to_a'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:324:in `find_first'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:117:in `first'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:282:in `find_one'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:274:in `find_with_ids'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:102:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/base.rb:403:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/associations.rb:1423:in `block in collection_accessor_methods'
    from (irb):16

I think it can be easily fixed:

Remove this "map(&:to_i)" from active_record/associations.rb:1422

def collection_accessor_methods(reflection, association_proxy_class, writer = true)
.
.
.
    define_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
      ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i)  # There is no need to map it to integer here in my opinion.
      send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids))
.
.
.

Comments and changes to this ticket

  • Neeraj Singh

    Neeraj Singh July 15th, 2010 @ 06:14 PM

    • Importance changed from “” to “Low”

    Can submit a patch using proper guidelines?

  • Robert Pankowecki

    Robert Pankowecki July 15th, 2010 @ 07:26 PM

    I will try :-)

  • Neeraj Singh

    Neeraj Singh July 15th, 2010 @ 08:51 PM

    • State changed from “new” to “open”

    This might not be a rails issue. Problem might line with Arel gem.

    I setup a model called 'brake' which has brake_id, a string, as primary key.

    I tested with arel and this is what I got:

    brakes = Table(:brakes)
brakes.where(brakes[:brake_id].eq('b1'))

# the sql that was generated is 
SELECT     "brakes"."brake_id", "brakes"."name", "brakes"."car_id" FROM       "brakes" WHERE     "brakes"."brake_id" = 0

    Looks like ARel does not like primary_key being string.

  • Robert Pankowecki

    Robert Pankowecki July 15th, 2010 @ 09:21 PM

    Sending patch and tests for this case.

  • Robert Pankowecki

    Robert Pankowecki July 15th, 2010 @ 09:26 PM

    Could not reproduce your statement about Arel

    ruby-1.9.2-head > Role.primary_key
 => "name"

ruby-1.9.2-head > role = Role.arel_table

ruby-1.9.2-head > role.where(role[:name].eq('role')).to_sql
 => "SELECT     \"roles\".\"name\", \"roles\".\"created_at\", \"roles\".\"updated_at\" FROM       \"roles\" WHERE     \"roles\".\"name\" = 'role'"
  • Neeraj Singh

    Neeraj Singh July 15th, 2010 @ 09:51 PM

    • Milestone set to “3.x”
    • Assigned user set to “José Valim”

    I had wrong settings.

    Your patch works.

    +1

  • Robert Pankowecki

    Robert Pankowecki July 19th, 2010 @ 06:22 PM

    I just found that my patch brokes the method when primary_key is a number. I'm goinh to post soon a better version.

  • Robert Pankowecki

    Robert Pankowecki July 19th, 2010 @ 06:24 PM

    But I have one question before: How should the method behave when the given list of ids constains unexisting id? Raise an exception or just ignore it silently?

  • José Valim

    José Valim July 21st, 2010 @ 02:27 PM

    Current behavior is to ignore silently, let's leave it that way. :)

  • Robert Pankowecki

    Robert Pankowecki July 27th, 2010 @ 09:04 PM

    I checked that current behavior is to raise an exception. It works like that for Rails 2.2.8 and Rails 3 so I will stay with current implementation which seems to more safe for me.

    Rails 2.3.8
ActiveRecord::RecordNotFound: Couldn't find all Containments with IDs (1,3) (found 1 results, but was looking for 2)
    from /home/rupert/.rvm/gems/ruby-1.9.2-head@rails222/gems/activerecord-2.3.8/lib/active_record/base.rb:1643:in `find_some'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head@rails222/gems/activerecord-2.3.8/lib/active_record/base.rb:1602:in `find_from_ids'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head@rails222/gems/activerecord-2.3.8/lib/active_record/base.rb:619:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head@rails222/gems/activerecord-2.3.8/lib/active_record/associations.rb:1337:in `block in collection_accessor_methods'

Rails 3.0.0.beta4
ActiveRecord::RecordNotFound: Couldn't find all Containments with IDs (1, 3) (found 0 results, but was looking for 2)
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:316:in `find_some'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:277:in `find_with_ids'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/relation/finder_methods.rb:102:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/base.rb:403:in `find'
    from /home/rupert/.rvm/gems/ruby-1.9.2-head/gems/activerecord-3.0.0.beta4/lib/active_record/associations.rb:1423:in `block in collection_accessor_methods'
  • José Valim

    José Valim July 27th, 2010 @ 09:15 PM

    Does this mean I can close this ticket?

  • Robert Pankowecki

    Robert Pankowecki July 27th, 2010 @ 09:39 PM

    No! I'm working on patch right at this moment to fix the behavior when the primary key is not integer. I will submit a patch in a seconds.

  • José Valim

    José Valim July 27th, 2010 @ 09:57 PM

    Ok! :)

  • Robert Pankowecki

    Robert Pankowecki July 27th, 2010 @ 10:01 PM

    I'm sending a better patch with test for string and integer primary keys (should work fine with other types of primary keys as well).

  • Krzysztof Kuczek

    Krzysztof Kuczek July 28th, 2010 @ 08:04 AM

    Hi,
    I've had similar problems, the patch seems to fix it. It just works. Thank you.

  • Repository

    Repository August 2nd, 2010 @ 03:55 PM

    • State changed from “open” to “resolved”

    (from [f8b53f35b9cbf2a134a7d9184a044ce95764acfa]) test and fix collection_singular_ids= with string primary keys [#5125 state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/f8b53f35b9cbf2a134a7d9184a044c...

  • Luis Correa d'Almeida

    Luis Correa d'Almeida August 9th, 2010 @ 01:51 AM

    I've just checked out the 3.0.0.rc3 and the following does not work:

    my_active_record_object.other_ids = ["uuid1", "uuid2"]

    It seems to still be casting those uuids to integers

    ActiveRecord::RecordNotFound: Couldn't find Other with ID=0

    Can anyone else confirm this?

  • Luis Correa d'Almeida

    Luis Correa d'Almeida August 9th, 2010 @ 01:55 AM

    And if you pass in a valid (existing) id, you get

    ActiveRecord::AssociationTypeMismatch: Other(#28595860) expected, got NilClass(#645570)

  • José Valim

    José Valim August 9th, 2010 @ 01:58 AM

    Could you please be more clear? There is no Rails 3.0.0.rc3.

  • Luis Correa d'Almeida

    Luis Correa d'Almeida August 9th, 2010 @ 02:57 AM

    Sorry, that was a typo - 3.0.0.rc

  • Luis Correa d'Almeida

    Luis Correa d'Almeida August 9th, 2010 @ 03:12 AM

    I checked the tag and it seems that it was tagged before this commit. So it makes sense that it is still breaking in the 3.0.0_RC tag.

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>

People watching this ticket

Attachments

Tags

Referenced by

Pages