Skip to content

Commit 529545a

Browse files
committed
Auto-enable identity primary keys in Migration[8.2]+
Phase 2 of #2579: build on the explicit `identity:` option from Phase 1 and switch the default for migrations declared at `ActiveRecord::Migration[8.2]` or later. Older migration versions, `Schema.define`, and direct `connection.create_table` keep the existing sequence + trigger behavior, so existing applications see no change unless they explicitly adopt `Migration[8.2]`. Wired through the new per-adapter extension point `AbstractAdapter#migration_compatibility_module_for(migration_class)` from yahonda/rails `another-33269` (not yet on rails master). The Gemfile temporarily points at that branch; once the Rails commit lands on `rails/rails#main` the Gemfile reference can be reverted. Architecture: two compat modules layered via Rails' versioned dispatch. `V8_2` forces `identity: true` for the current default; `V8_1` forces `identity: false` to preserve legacy behavior on older migrations. The adapter's underlying `create_table` honors `options[:identity]` exactly as given and does no auto-injection, so callers outside the migration compat chain (`Schema.define`, direct `connection.create_table`) keep sequence-backed PKs by default. `db/schema.rb` roundtrips remain safe because the schema dumper now emits `identity: true` / `identity: false` explicitly for every primary-key table. `V8_2` also treats explicit Oracle sequence options as opt-out: when `sequence_name:` or `sequence_start_value:` is given, the requested sequence is created and the PK stays sequence-backed. `V8_2` pre-checks `connection.supports_identity_columns?` so `Migration[8.2]+` on Oracle <12.1 silently falls back to the legacy sequence path instead of triggering the Phase 1 ArgumentError. Per-table opt-out remains via `identity: false`; per-migration opt-out via `Migration[8.1]` or earlier. `rename_table` and `drop_table` are now identity-aware: they no longer rename or drop `<table>_seq` for tables whose primary key is an Oracle identity column. Previously, an unrelated application-owned sequence sharing that name would be silently mutated or removed when the table was renamed/dropped.
1 parent 30d002a commit 529545a

8 files changed

Lines changed: 376 additions & 6 deletions

File tree

Gemfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ group :development do
88
gem "rspec"
99
gem "rdoc"
1010
gem "rake"
11-
gem "activerecord", github: "rails/rails", branch: "main"
11+
gem "activerecord", github: "yahonda/rails", branch: "another-33269"
1212
gem "ruby-plsql", github: "rsim/ruby-plsql", branch: "master"
1313

1414
platforms :ruby do
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# frozen_string_literal: true
2+
3+
module ActiveRecord
4+
module ConnectionAdapters
5+
module OracleEnhanced
6+
module MigrationCompatibility # :nodoc: all
7+
extend ActiveRecord::Migration::Compatibility::Versioned
8+
9+
module V8_2
10+
def create_table(table_name, **options)
11+
options[:identity] = true if oracle_enhanced_default_to_identity?(options)
12+
super
13+
end
14+
15+
private
16+
def oracle_enhanced_default_to_identity?(options)
17+
return false if options.key?(:identity)
18+
return false if options[:sequence_name] || options[:sequence_start_value]
19+
return false if options.fetch(:id, :primary_key) != :primary_key
20+
return false if options[:primary_key].is_a?(Array)
21+
connection.supports_identity_columns?
22+
end
23+
end
24+
25+
module V8_1
26+
def create_table(table_name, **options)
27+
options[:identity] = false unless options.key?(:identity)
28+
super
29+
end
30+
end
31+
end
32+
end
33+
end
34+
end

lib/active_record/connection_adapters/oracle_enhanced/schema_dumper.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ def table(table, stream)
122122
end
123123
tbl.print ", #{format_colspec(pkcolspec)}"
124124
end
125-
tbl.print ", identity: true" if pkcol.auto_incremented_by_db?
125+
if pkcol.auto_incremented_by_db?
126+
tbl.print ", identity: true"
127+
elsif @connection.supports_identity_columns?
128+
tbl.print ", identity: false"
129+
end
126130
when Array
127131
tbl.print ", primary_key: #{pk.inspect}"
128132
else

lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,11 @@ def rename_table(table_name, new_name, **options) # :nodoc:
235235
end
236236
schema_cache.clear_data_source_cache!(table_name.to_s)
237237
schema_cache.clear_data_source_cache!(new_name.to_s)
238+
identity_pk = identity_pk?(table_name)
238239
execute "RENAME #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"
239-
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
240+
unless identity_pk
241+
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
242+
end
240243
@prefetch_primary_key_cache.delete(table_name.to_s)
241244

242245
rename_table_indexes(table_name, new_name, **options)
@@ -248,9 +251,13 @@ def drop_table(*table_names, **options) # :nodoc:
248251
custom_sequence_name = table_names.size == 1 ? options[:sequence_name] : nil
249252
table_names.each do |table_name|
250253
schema_cache.clear_data_source_cache!(table_name.to_s)
254+
identity_pk = identity_pk?(table_name)
251255
execute "DROP TABLE #{quote_table_name(table_name)}#{' CASCADE CONSTRAINTS' if options[:force] == :cascade}"
252-
seq_name = custom_sequence_name || default_sequence_name(table_name, nil)
253-
execute "DROP SEQUENCE #{quote_table_name(seq_name)}" rescue nil
256+
if custom_sequence_name
257+
execute "DROP SEQUENCE #{quote_table_name(custom_sequence_name)}" rescue nil
258+
elsif !identity_pk
259+
execute "DROP SEQUENCE #{quote_table_name(default_sequence_name(table_name, nil))}" rescue nil
260+
end
254261
rescue ActiveRecord::StatementInvalid => e
255262
raise e unless options[:if_exists]
256263
ensure
@@ -623,6 +630,18 @@ def valid_primary_key_options # :nodoc:
623630
end
624631

625632
private
633+
# True when +table_name+ exists and its primary key column is an Oracle
634+
# identity column. Returns false when the table is not present (e.g.
635+
# +drop_table if_exists:+ for a missing table) or when the database does
636+
# not support identity columns.
637+
def identity_pk?(table_name)
638+
return false unless supports_identity_columns?
639+
owner, desc_table_name = resolve_data_source_name(table_name)
640+
identity_primary_key?(owner, desc_table_name)
641+
rescue OracleEnhanced::ConnectionException
642+
false
643+
end
644+
626645
def validate_identity_options!(identity, id, primary_key)
627646
return unless identity
628647
unless supports_identity_columns?

lib/active_record/connection_adapters/oracle_enhanced_adapter.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
require "active_record/connection_adapters/oracle_enhanced/deprecator"
3939
require "active_record/connection_adapters/oracle_enhanced/connection"
4040
require "active_record/connection_adapters/oracle_enhanced/database_statements"
41+
require "active_record/connection_adapters/oracle_enhanced/migration_compatibility"
4142
require "active_record/connection_adapters/oracle_enhanced/schema_creation"
4243
require "active_record/connection_adapters/oracle_enhanced/schema_definitions"
4344
require "active_record/connection_adapters/oracle_enhanced/schema_dumper"
@@ -383,6 +384,10 @@ def supports_identity_columns? # :nodoc:
383384
database_version.first >= 12
384385
end
385386

387+
def migration_compatibility_module_for(migration_class) # :nodoc:
388+
OracleEnhanced::MigrationCompatibility.module_for(migration_class)
389+
end
390+
386391
def supports_json?
387392
# Oracle Database 12.1 or higher version supports JSON.
388393
# However, Oracle enhanced adapter has limited support for JSON data type.

spec/active_record/connection_adapters/oracle_enhanced/identity_primary_key_spec.rb

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ def identity_column_exists?(table_name, column_name)
322322
ActiveRecord::SchemaDumper.ignore_tables = []
323323

324324
expect(stream.string).to include('create_table "test_identity_pks"')
325+
expect(stream.string).to include("identity: false")
325326
expect(stream.string).not_to include("identity: true")
326327
end
327328
end
@@ -333,4 +334,111 @@ def identity_column_exists?(table_name, column_name)
333334
}.to raise_error(NotImplementedError)
334335
end
335336
end
337+
338+
describe "identity counterparts to sequence-prerequisite behavior" do
339+
before do
340+
skip "requires Oracle 12.1+" unless @oracle12c_or_higher
341+
end
342+
343+
it "renames a long-named identity table without touching a non-existent sequence" do
344+
long_source = "a" * (@conn.max_identifier_length - 3)
345+
schema_define do
346+
create_table long_source.to_sym, force: true, identity: true do |t|
347+
t.string :name
348+
end
349+
end
350+
351+
expected_old_seq = @conn.default_sequence_name(long_source, nil).upcase
352+
expected_new_seq = @conn.default_sequence_name("renamed_identity_long", nil).upcase
353+
354+
expect {
355+
@conn.rename_table(long_source, "renamed_identity_long")
356+
}.not_to raise_error
357+
358+
sequences = @conn.select_values(
359+
"SELECT sequence_name FROM user_sequences WHERE sequence_name IN ('#{expected_old_seq}', '#{expected_new_seq}')"
360+
)
361+
expect(sequences).to be_empty
362+
ensure
363+
schema_define do
364+
drop_table :renamed_identity_long, if_exists: true
365+
drop_table long_source.to_sym, if_exists: true
366+
end
367+
end
368+
369+
it "returns a nil sequence_name from pk_and_sequence_for for identity tables" do
370+
schema_define do
371+
create_table :test_identity_pks, identity: true do |t|
372+
t.string :name
373+
end
374+
end
375+
376+
expect(@conn.pk_and_sequence_for(:test_identity_pks)).to eq(["id", nil])
377+
end
378+
379+
it "does not query NEXTVAL when inserting into an identity table" do
380+
schema_define do
381+
create_table :test_identity_pks, identity: true do |t|
382+
t.string :name
383+
end
384+
end
385+
klass = Class.new(ActiveRecord::Base) { self.table_name = "test_identity_pks" }
386+
387+
set_logger
388+
begin
389+
klass.create!(name: "alpha")
390+
klass.create!(name: "bravo")
391+
expect(@logger.output(:debug)).not_to match(/NEXTVAL/i)
392+
ensure
393+
clear_logger
394+
end
395+
end
396+
397+
it "omits DROP SEQUENCE from full_drop output for identity-backed tables" do
398+
schema_define do
399+
create_table :test_identity_pks, identity: true do |t|
400+
t.string :name
401+
end
402+
end
403+
404+
drop = @conn.full_drop
405+
406+
expect(drop).to match(/DROP TABLE "TEST_IDENTITY_PKS" CASCADE CONSTRAINTS/)
407+
expect(drop).not_to match(/DROP SEQUENCE "TEST_IDENTITY_PKS_SEQ"/)
408+
end
409+
410+
it "rename_table leaves an unrelated <table>_seq sequence alone for identity tables" do
411+
schema_define do
412+
create_table :test_identity_pks, identity: true do |t|
413+
t.string :name
414+
end
415+
end
416+
@conn.execute "CREATE SEQUENCE test_identity_pks_seq"
417+
418+
@conn.rename_table(:test_identity_pks, :test_identity_pks_renamed)
419+
420+
expect(sequence_exists?(:test_identity_pks_seq)).to be true
421+
expect(sequence_exists?(:test_identity_pks_renamed_seq)).to be false
422+
ensure
423+
@conn.execute("DROP SEQUENCE test_identity_pks_seq") rescue nil
424+
schema_define do
425+
drop_table :test_identity_pks_renamed, if_exists: true
426+
end
427+
end
428+
429+
it "drop_table leaves an unrelated <table>_seq sequence alone for identity tables" do
430+
schema_define do
431+
create_table :test_identity_pks, identity: true do |t|
432+
t.string :name
433+
end
434+
end
435+
@conn.execute "CREATE SEQUENCE test_identity_pks_seq"
436+
437+
@conn.drop_table(:test_identity_pks)
438+
439+
expect(sequence_exists?(:test_identity_pks_seq)).to be true
440+
ensure
441+
@conn.execute("DROP SEQUENCE test_identity_pks_seq") rescue nil
442+
end
443+
end
336444
end

0 commit comments

Comments
 (0)