Allow auto_increment columns to be part of composite key (MySQL only for now) #147

Merged
merged 1 commit into from Apr 14, 2013

2 participants

@krbullock

As I posted in https://groups.google.com/forum/?fromgroups=#!topic/compositekeys/PES6O2B-dWI, creating a composite key that has an auto-increment field as part of it doesn't work as expected. Here's a patch that fixes this on MySQL, with a test case. It's a one-line monkeypatch of ActiveRecord::Persistence#create to assign to self[:id] instead of self.id.

If this line of development is interesting, I can look into fixing it on PostgreSQL and SQLite3 as well.

@cfis

Thanks for the patch. So definitely interested, but how does this work? Is there one sequence per column? Does mysql support autogenerating keys across columns?

@krbullock

MySQL can only have one auto_increment column per table. In the usual Rails+MySQL happy path, you make your 'id' column the auto_increment. When Rails creates a new record (in AR::Persistence#create), it reads the last-inserted auto_increment back from the database and assigns it to the object it just persisted. Reference for auto_increment here: http://dev.mysql.com/doc/refman/5.5/en/example-auto-increment.html and here: http://dev.mysql.com/doc/refman/5.5/en/create-table.html.

Since the PostgreSQL adapter pre-fetches the next ID in the sequence, the fix will have to work a bit differently. I haven't looked into it far enough yet to know exactly how.

@cfis

Some more questions. The create method you have doesn't match the latest in AR 3.2.11 - we should always use the latest. Also, doesn't seeting the id not work since its a single value and this is a CPK model?

self.id = connection.next_sequence_value(self.class.sequence_name)

@krbullock

Right, I made this change on the Rails 3.0.x compatible branch, and thus pulled the code from the latest on their 3-0-stable branch. I can incorporate my changes onto ar_3.1.x and master by pulling code from the respective Rails branches too.

Setting the ID manually works as of this change because I changed:

self.id = ...

to:

self[:id] = ...

thus bypassing the CPK override of #id=.

@cfis

Ah - sorry missed the AR branch.

So some more questions though:

  1. What about the first three lines of the create method and their use of self.id. Shouldn't those change?
  2. What if the model doesn't actually have an id field, what happens?
@krbullock
  1. For now (just MySQL), the first three lines don't matter because connection.prefetch_primary_key? will always return false. That would need to be updated when adding support for other DBMSes.

  2. Probably a colorful explosion. I'll look into this.

@cfis

Status?

@cfis cfis merged commit 3c46ffb into composite-primary-keys:ar_3.0.x Apr 14, 2013
@cfis

Thanks - sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment