Rails: Changing a model's `primary_key` breaks generated SQL.

[kitlangton]

Steps to reproduce

1. rails new example --database=postgresql
2. rails g model Thing uid flag:boolean
3. Add self.primary_key = 'uid' toapp/models/thing.rb
4. Create a Thing instance w/ a uid and then attempt to update it.

Expected behavior

After updating, I'd expect the value to have updated successfully.

Actual behavior

Using rails console, I take a Thing I've created and attempt to update it. The update appears to be successful, but once I reload my Thing instance, it becomes clear that the update didn't stick.

2.4.0 :002 > t = Thing.first
  Thing Load (0.4ms)  SELECT  "things".* FROM "things" ORDER BY "things"."uid" ASC LIMIT $1  [["LIMIT", 1]]
 => #<Thing id: "HI", uid: "HI", flag: nil, created_at: "2017-06-03 17:15:06", updated_at: "2017-06-03 17:15:06">
2.4.0 :003 > t.update(flag: true)
   (0.3ms)  BEGIN
  SQL (1.1ms)  UPDATE "things" SET "flag" = $1, "updated_at" = $2 WHERE "things"."uid" = $3  [["flag", "t"], ["updated_at", "2017-06-03 17:28:35.485354"], ["uid", "1"]]
   (0.4ms)  COMMIT
 => true
2.4.0 :004 > t
 => #<Thing id: "HI", uid: "HI", flag: true, created_at: "2017-06-03 17:15:06", updated_at: "2017-06-03 17:28:35">
2.4.0 :005 > t.reload
  Thing Load (0.5ms)  SELECT  "things".* FROM "things" WHERE "things"."uid" = $1 LIMIT $2  [["uid", "HI"], ["LIMIT", 1]]
 => #<Thing id: "HI", uid: "HI", flag: nil, created_at: "2017-06-03 17:15:06", updated_at: "2017-06-03 17:15:06">

The flag property returns to nil after reloading. You can see that the call to update is trying to find the record w/ ["uid", "1"], which is the original serial id, and not the uid field I specified.

This worked before I updated to Rails 5 :-(

Additionally, if I attempt to validate uniqueness on :uid, then I can't even save existing records. ActiveRecord ends up finds the self-same record in the database and doesn't realize they're equal. However, I believe this stems from the same problem of mixing up the primary_key (which was set to uid) and the default id.

System configuration

Rails version: 5.1.1

Ruby version: 2.4.0

[jakeonfire]

I'm having the same issue. Wondering if you've found a workaround. The id is an integer, but the primary_key is a string, and it's trying to use the id value as the primary_key to reference the record for updates. Model:

# Table name: people
#
#  id         :integer          not null
#  orn        :string           not null, primary key
#  notes      :text
#

class Person < ActiveRecord::Base
  self.primary_key = :orn
  attr_readonly :orn

> person = Person.find 'ORN00008079837'
  Person Load (0.2ms)  SELECT  "people".* FROM "people" WHERE "people"."orn" = $1 LIMIT $2  [["orn", "ORN00008079837"], ["LIMIT", 1]]
> person.update! notes: nil
   (0.1ms)  BEGIN
  SQL (0.4ms)  UPDATE "people" SET "updated_at" = $1, "notes" = $2 WHERE "people"."orn" = $3  [["updated_at", "2017-06-06 21:41:13.814079"], ["notes", nil], ["orn", "8079849"]]
   (1.4ms)  COMMIT
=> true

so Rails thinks it succeeded but the DB isn't updated since the WHERE clause doesn't match any records. in the database, id is indeed 8079849 but in Rails id is ORN00008079837...

[jakeonfire]

i found a workaround for updates. maybe it will help with understanding the underlying issue:

def id_in_database
  self[self.class.primary_key]
end

i was looking at the following source and guessing what happens next:
activerecord-5.1.0/lib/active_record/persistence.rb:575

[eugeneius]

This worked before I updated to Rails 5 :-(

I was intrigued by this, as the proposed fix (https://github.com/rails/rails/pull/29378) changes code that has been around for a lot longer than that. I turned the new test from that PR into a repro script and bisected the failure.

It started to fail at https://github.com/rails/rails/commit/16ae3db5a5c6a08383b974ae6c96faac5b4a3c81, when id_was was replaced with id_in_database here. Checking out that commit and reverting that one line made the test pass.

However, making the same change on master didn't fix the test. I tracked it down to https://github.com/rails/rails/commit/b5eb3215a68f94bb8cb20739366232c415744b83, which changed the id_was method to use _read_attribute instead of going through the id method and picking up the primary key.

To summarise: id_was used to always read the primary key, id_in_database was introduced and always read the id column instead, and then id_was was changed to behave like id_in_database. https://github.com/rails/rails/pull/29378 makes both methods work like id_was did pre-5.1.