Skip to content

Commit 7741715

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 a19a59e commit 7741715

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: "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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ def table(table, stream)
129129
tbl.print ", primary_key_trigger: true"
130130
default_name = @connection.default_trigger_name(table).upcase
131131
tbl.print ", trigger_name: #{trigger_name.downcase.inspect}" unless trigger_name == default_name
132+
elsif @connection.supports_identity_columns?
133+
tbl.print ", identity: false"
132134
end
133135
when Array
134136
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
@@ -239,8 +239,11 @@ def rename_table(table_name, new_name, **options) # :nodoc:
239239
end
240240
schema_cache.clear_data_source_cache!(table_name.to_s)
241241
schema_cache.clear_data_source_cache!(new_name.to_s)
242+
identity_pk = identity_pk?(table_name)
242243
execute "RENAME #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"
243-
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
244+
unless identity_pk
245+
execute "RENAME #{default_sequence_name(table_name, nil)} TO #{default_sequence_name(new_name, nil)}" rescue nil
246+
end
244247
rename_pk_trigger(table_name, new_name)
245248
@prefetch_primary_key_cache.delete(table_name.to_s)
246249
clear_table_columns_cache(table_name)
@@ -258,10 +261,13 @@ def drop_table(*table_names, **options) # :nodoc:
258261

259262
table_names.each do |table_name|
260263
schema_cache.clear_data_source_cache!(table_name.to_s)
261-
seq_name = custom_sequence_name || default_sequence_name(table_name, nil)
264+
# Identity-backed tables never carry a default `<table>_seq`, so
265+
# skip the sequence drop entirely on those. `identity_pk?` must
266+
# run before the table drop, while the table is still present.
267+
seq_name = custom_sequence_name || (identity_pk?(table_name) ? nil : default_sequence_name(table_name, nil))
262268

263269
drop_if_exists("TABLE", table_name, cascade_constraints: cascade, if_exists: if_exists)
264-
drop_if_exists("SEQUENCE", seq_name, if_exists: true)
270+
drop_if_exists("SEQUENCE", seq_name, if_exists: true) if seq_name
265271
ensure
266272
clear_table_columns_cache(table_name)
267273
@prefetch_primary_key_cache.delete(table_name.to_s)
@@ -704,6 +710,18 @@ def missing_object_ora_code?(message, object_type)
704710
code && message.include?(code)
705711
end
706712

713+
# True when +table_name+ exists and its primary key column is an Oracle
714+
# identity column. Returns false when the table is not present (e.g.
715+
# +drop_table if_exists:+ for a missing table) or when the database does
716+
# not support identity columns.
717+
def identity_pk?(table_name)
718+
return false unless supports_identity_columns?
719+
owner, desc_table_name = resolve_data_source_name(table_name)
720+
identity_primary_key?(owner, desc_table_name)
721+
rescue OracleEnhanced::ConnectionException
722+
false
723+
end
724+
707725
def index_column_names(column_names) # :nodoc:
708726
column_names.is_a?(Array) ? column_names : Array(column_names)
709727
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"
@@ -539,6 +540,10 @@ def use_dbms_metadata_dump? # :nodoc:
539540
database_version >= "12.1"
540541
end
541542

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

0 commit comments

Comments
 (0)