Skip to content

Commit ba8c016

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 `per-adapter-migration-compatibility` (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 429aa95 commit ba8c016

8 files changed

Lines changed: 372 additions & 5 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: "per-adapter-migration-compatibility"
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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ def table(table, stream)
187187
tbl.print ", primary_key_trigger: true"
188188
default_name = @connection.default_trigger_name(table).upcase
189189
tbl.print ", trigger_name: #{trigger_name.downcase.inspect}" unless trigger_name == default_name
190+
elsif @connection.supports_identity_columns?
191+
tbl.print ", identity: false"
190192
end
191193
when Array
192194
tbl.print ", primary_key: #{pk.inspect}"

lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,11 @@ def rename_table(table_name, new_name, **options) # :nodoc:
263263
end
264264
schema_cache.clear_data_source_cache!(table_name.to_s)
265265
schema_cache.clear_data_source_cache!(new_name.to_s)
266+
identity_pk = identity_pk?(table_name)
266267
execute "RENAME #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"
267-
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
268+
unless identity_pk
269+
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
270+
end
268271
rename_pk_trigger(table_name, new_name)
269272
clear_table_caches(table_name)
270273
clear_table_caches(new_name)
@@ -281,10 +284,13 @@ def drop_table(*table_names, **options) # :nodoc:
281284

282285
table_names.each do |table_name|
283286
schema_cache.clear_data_source_cache!(table_name.to_s)
284-
seq_name = custom_sequence_name || default_sequence_name(table_name, nil)
287+
# Identity-backed tables never carry a default `<table>_seq`, so
288+
# skip the sequence drop entirely on those. `identity_pk?` must
289+
# run before the table drop, while the table is still present.
290+
seq_name = custom_sequence_name || (identity_pk?(table_name) ? nil : default_sequence_name(table_name, nil))
285291

286292
drop_if_exists("TABLE", table_name, cascade_constraints: cascade, if_exists: if_exists)
287-
drop_if_exists("SEQUENCE", seq_name, if_exists: true)
293+
drop_if_exists("SEQUENCE", seq_name, if_exists: true) if seq_name
288294
ensure
289295
clear_table_caches(table_name)
290296
end
@@ -1215,6 +1221,18 @@ def missing_object_ora_code?(message, object_type)
12151221
code && message.include?(code)
12161222
end
12171223

1224+
# True when +table_name+ exists and its primary key column is an Oracle
1225+
# identity column. Returns false when the table is not present (e.g.
1226+
# +drop_table if_exists:+ for a missing table) or when the database does
1227+
# not support identity columns.
1228+
def identity_pk?(table_name)
1229+
return false unless supports_identity_columns?
1230+
owner, desc_table_name = resolve_data_source_name(table_name)
1231+
identity_primary_key?(owner, desc_table_name)
1232+
rescue OracleEnhanced::ConnectionException
1233+
false
1234+
end
1235+
12181236
def index_column_names(column_names) # :nodoc:
12191237
column_names.is_a?(Array) ? column_names : Array(column_names)
12201238
end

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"
@@ -619,6 +620,10 @@ def use_dbms_metadata_dump? # :nodoc:
619620
database_version >= "12.1"
620621
end
621622

623+
def migration_compatibility_module_for(migration_class) # :nodoc:
624+
OracleEnhanced::MigrationCompatibility.module_for(migration_class)
625+
end
626+
622627
def supports_json?
623628
# Oracle Database 12.1 or higher version supports JSON.
624629
# 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
@@ -336,6 +336,7 @@ def identity_column_exists?(table_name, column_name)
336336
ActiveRecord::SchemaDumper.ignore_tables = []
337337

338338
expect(stream.string).to include('create_table "test_identity_pks"')
339+
expect(stream.string).to include("identity: false")
339340
expect(stream.string).not_to include("identity: true")
340341
end
341342
end
@@ -352,4 +353,111 @@ def identity_column_exists?(table_name, column_name)
352353
}.to raise_error(NotImplementedError)
353354
end
354355
end
356+
357+
describe "identity counterparts to sequence-prerequisite behavior" do
358+
before do
359+
skip "requires Oracle 12.1+" unless @oracle12c_or_higher
360+
end
361+
362+
it "renames a long-named identity table without touching a non-existent sequence" do
363+
long_source = "a" * (@conn.max_identifier_length - 3)
364+
schema_define do
365+
create_table long_source.to_sym, force: true, identity: true do |t|
366+
t.string :name
367+
end
368+
end
369+
370+
expected_old_seq = @conn.default_sequence_name(long_source, nil).upcase
371+
expected_new_seq = @conn.default_sequence_name("renamed_identity_long", nil).upcase
372+
373+
expect {
374+
@conn.rename_table(long_source, "renamed_identity_long")
375+
}.not_to raise_error
376+
377+
sequences = @conn.select_values(
378+
"SELECT sequence_name FROM user_sequences WHERE sequence_name IN ('#{expected_old_seq}', '#{expected_new_seq}')"
379+
)
380+
expect(sequences).to be_empty
381+
ensure
382+
schema_define do
383+
drop_table :renamed_identity_long, if_exists: true
384+
drop_table long_source.to_sym, if_exists: true
385+
end
386+
end
387+
388+
it "returns a nil sequence_name from pk_and_sequence_for for identity tables" do
389+
schema_define do
390+
create_table :test_identity_pks, identity: true do |t|
391+
t.string :name
392+
end
393+
end
394+
395+
expect(@conn.pk_and_sequence_for(:test_identity_pks)).to eq(["id", nil])
396+
end
397+
398+
it "does not query NEXTVAL when inserting into an identity table" do
399+
schema_define do
400+
create_table :test_identity_pks, identity: true do |t|
401+
t.string :name
402+
end
403+
end
404+
klass = Class.new(ActiveRecord::Base) { self.table_name = "test_identity_pks" }
405+
406+
set_logger
407+
begin
408+
klass.create!(name: "alpha")
409+
klass.create!(name: "bravo")
410+
expect(@logger.output(:debug)).not_to match(/NEXTVAL/i)
411+
ensure
412+
clear_logger
413+
end
414+
end
415+
416+
it "omits DROP SEQUENCE from full_drop output for identity-backed tables" do
417+
schema_define do
418+
create_table :test_identity_pks, identity: true do |t|
419+
t.string :name
420+
end
421+
end
422+
423+
drop = @conn.full_drop
424+
425+
expect(drop).to match(/DROP TABLE "TEST_IDENTITY_PKS" CASCADE CONSTRAINTS/)
426+
expect(drop).not_to match(/DROP SEQUENCE "TEST_IDENTITY_PKS_SEQ"/)
427+
end
428+
429+
it "rename_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.rename_table(:test_identity_pks, :test_identity_pks_renamed)
438+
439+
expect(sequence_exists?(:test_identity_pks_seq)).to be true
440+
expect(sequence_exists?(:test_identity_pks_renamed_seq)).to be false
441+
ensure
442+
@conn.execute("DROP SEQUENCE test_identity_pks_seq") rescue nil
443+
schema_define do
444+
drop_table :test_identity_pks_renamed, if_exists: true
445+
end
446+
end
447+
448+
it "drop_table leaves an unrelated <table>_seq sequence alone for identity tables" do
449+
schema_define do
450+
create_table :test_identity_pks, identity: true do |t|
451+
t.string :name
452+
end
453+
end
454+
@conn.execute "CREATE SEQUENCE test_identity_pks_seq"
455+
456+
@conn.drop_table(:test_identity_pks)
457+
458+
expect(sequence_exists?(:test_identity_pks_seq)).to be true
459+
ensure
460+
@conn.execute("DROP SEQUENCE test_identity_pks_seq") rescue nil
461+
end
462+
end
355463
end

0 commit comments

Comments
 (0)