rsim / oracle-enhanced

Oracle enhaced adapter for ActiveRecord
MIT License
549 stars 310 forks source link

Optional identity columns #2201

Open brianthoman opened 3 years ago

brianthoman commented 3 years ago

The goal here is to add optional support for Oracle 12's IDENTITY columns.

This code is probably still in need of some cleanup/adjustments, but I wanted to submit it as a sort of proof-of-concept and get some feedback before I continue working on it.

By default, IDENTITY columns are disabled. They can be enabled in the following ways:

For the whole app via initializer/config:

ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.use_identity_for_pk = true

In migrations:

create_table :things, primary_key_as_identity: true

# OR

create_table :things, id: false do |t|
  t.primary_key :test_id, :primary_key, identity: true
end

# OR

add_column :things, :test_id, :primary_key, identity: true

Please let me know what you think. Thank you!

mattalat commented 2 years ago

We would love this change. It also would solve https://github.com/rsim/oracle-enhanced/issues/2049.

akostadinov commented 2 years ago

If Rails can create proper relationships with the new column type, then I would say, use it by default unless user requested older style primary key. Or course some test cases to show that would be needed.

Another thing to consider is migrations and schema dumps with :ruby format. For migrations I don't know, but probably it makes sense to dump selecting old style PK for tables using old style.

In either case, thorough upgrade docs would be needed.

mattalat commented 2 years ago

@yahonda Sorry to ping directly, but I wonder if you have any reservations about this PR or any guidance on things to improve so it can be accepted. I would love to see this support make it into the gem.

yahonda commented 2 years ago

Let me have some review.