This project is archived and is in readonly mode.

Regression: mass assignment interaction with a non-standard primary key (2.3.5 -> 2.3.10)

#6337

We had to patch a year old app today that was running happily on 2.3.5. As part of the work, we did a routine upgrade to 2.3.10 and found a weird bug when using non-standard primary keys.

You can replicate this way. Create a new Rails app with 2.3.10 and then add these two files:

# app/models/country.rb
class Country < ActiveRecord::Base
  set_primary_key :iso
end

# db/migrations/20110126_create_countries.db
class CreateCountries < ActiveRecord::Migration
  def self.up
    create_table :countries, :id => false, :primary_key => :iso do |t|
      t.string :iso, :limit => 2, :null => false
      t.string :name
      t.integer :region_id

      t.timestamps
    end 

    execute "ALTER TABLE countries ADD PRIMARY KEY (iso);"
  end

  def self.down
    drop_table :countries
  end
end

Then create your database (you'll need to use MySQL or Postgres, not SQLite) and migrate it.

The following operations fail to set ID correctly:

# Create
Country.create(:iso => "AU", :name => "Australia")

# New & Save
c = Country.new(:iso => "AU", :name => "Australia")
c.save

This is the only way to set the iso column:

# Working #1
c = Country.new(:name => "Australia")
c.iso = "AU"
c.save

If you don't do this your primary key columns will be populated with the default value. Rails itself appears to be simply omitting the :iso column from the query:

INSERT INTO `countries` (`name`, `created_at`, `updated_at`, `region_id`)
VALUES('Australia', '2011-01-26 19:47:57', '2011-01-26 19:47:57', NULL)

These migrations worked under 2.3.5.

I don't know enough about ActiveRecord to find the location in the code but it appears it's making an assumption that the primary key cannot be mass assigned, and this change was introduced at some point between 2.3.5 and 2.3.10.

Reported by mat · January 26th, 2011 @ 08:04 PM

Activity

Cesario

I'm afraid this is not a bug, and it can be reproduced on 2.3.5, 2.3.10 or 3.0.3.
Primary keys are marked as protected, this test case will not fail:

class Subscriber < AR::Base
  set_primary_key :nick
  attr_accessible :nick # Mark it available for mass-assignment
end

def test_customized_string_primary_key_set_with_create
  subscriber = Subscriber.create :nick => 'webster123'
  assert_equal 'webster123', subscriber.id
  assert_equal 'webster123', subscriber.nick
end

Mass-assigning the key is just prevented unless you set it explicitly accessible.

Even on 2.3.5, if I remove the attr_accessible line, i'm getting this result for the same test:

1) Error:
test_customized_string_primary_key_set_with_create(PrimaryKeysTest):
ActiveRecord::StatementInvalid: SQLite3::ConstraintException: subscribers.nick may not be NULL: INSERT INTO "subscribers" ("name") VALUES(NULL)

January 30th, 2011 @ 10:38 AM

mat
I get that primary keys are protected, I don't get how the migration ever worked a year ago. Something's changed somewhere.

I'll add the attr_accessible and we can all move on!

Thanks

M.

January 30th, 2011 @ 09:56 PM
Cesario
I won't jump into the code right now, but you could check in 2.3.5, at runtime, if the iso column has been set automatically attr_protected when you ran set_primary_key.

In any case, I think we could close this ticket. I hope it helped :)

January 30th, 2011 @ 10:06 PM
mat
Yes, let's close it. Your explanation about why primary_keys are by default protected was very helpful and makes sense.

I wish there was somewhere we could document that you need to open up the primary key for mass assignment if the purpose is to set it yourself. All the examples I found were to use things like sequences in Postgres, etc.

Thanks

January 30th, 2011 @ 10:11 PM
Rohit Arondekar
- State changed from new to invalid
- Importance changed from to Low
February 1st, 2011 @ 05:44 AM