Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

ActiveRecord 3.1 cannot load models with non-default PKs unless there's already a database connection #2807

Closed
rsutphin opened this Issue · 5 comments

3 participants

@rsutphin

Using ActiveRecord 3.1, one cannot load a model class that calls set_primary_key unless there is already a database connection. This works with ActiveRecord 2.3 and 3.0. I've created a minimal test case to demonstrate the issue.

The failure looks like this:

/Users/rsutphin/.rvm/gems/ruby-1.9.2-p290@tmp/gems/activerecord-3.1.0/lib/active_record/attribute_methods/primary_key.rb:69:in `set_primary_key': undefined method `primary_keys' for nil:NilClass (NoMethodError)
from /private/tmp/ar-cp-pk/models.rb:4:in `<class:Person>'
from /private/tmp/ar-cp-pk/models.rb:3:in `<top (required)>'
from example.rb:8:in `require'
from example.rb:8:in `load_models'
from example.rb:28:in `<main>'
@jonleighton
Collaborator

@tenderlove I have fixed in the above commit. It's not the nicest fix though.

I think in the future we should have connection_pool.primary_keys only cache the primary key names as reported by the database. Then, in AR::Base#columns, we should clone (and cache) connection_pool.columns[table_name] value and set the appropriate one to have primary = true. This would decouple tables from models - at the moment we are assuming there is a 1-1 mapping between models and tables.

@kjg

This change causes another issue. When you run a query such as #create, this causes the column definitions to be cached before #columns is called. Thus the columns are cached with none of them acting as the primary key.

connection_pool.primary_keys[table_name] = value needs to happen BEFORE #relation is called

@kjg

This new version of #columns works around the problem. https://gist.github.com/1410534

@jonleighton jonleighton reopened this
@jonleighton jonleighton was assigned
@jonleighton
Collaborator

I cannot repro with this script, but I am not sure if I am doing the right thing to trigger the issue. Please could you confirm if this is still an issue on current master. (Run the script with bundle exec ruby /path/to/bug_2807.rb.) If it is, please open a new bug with a script attached to repro the issue. Thanks.

@kjg

I've updated my gist to add a failing test. I've also opened a new issue describing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.