Skip to content

Commit bcdd815

Browse files
yahondaclaude
andcommitted
Phase 2: flip add_index unique: true default in Migration[8.2]+
Closes #2702 (Phase 2). Depends on Rails core extension yahonda/rails branch:per-adapter-migration-compatibility (`migration_compatibility_module_for`). In Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation, the global default flips from "implicit constraint + warning" (Phase 1) to "create the unique index only", matching Rails-core PostgreSQL/MySQL/SQLite. Existing migrations declared at `Migration[8.1]` or earlier opt back into the legacy implicit-constraint behavior via a `MigrationCompatibility::V8_1` module so they keep working unchanged. * `OracleEnhancedAdapter.add_index_unique_creates_constraint` default flips from `true` to `false`. Set to `true` explicitly to force the legacy behavior project-wide. * New `OracleEnhanced::MigrationCompatibility::V8_1` module overrides `add_index` and `create_table` to set a private thread-local that the adapter consults, opting old migrations back into the implicit- constraint path. The thread-local is encapsulated as a private constant of `MigrationCompatibility` so Phase 3 can remove the entire module — key included — without leaving observable state behind in `Thread.current`. * New `migration_compatibility_module_for(migration_class)` on `OracleEnhancedAdapter` wires the compat module into Rails' Migration dispatch. * Gemfile pins `activerecord` to `yahonda/rails branch:per-adapter-migration-compatibility` for the duration of Phase 2 (will be reverted to `rails/rails main` once the branch is merged). Specs: * New `migration_compatibility_spec.rb` covering Migration[8.2] (no implicit constraint, no warning), Migration[8.1] (implicit constraint + warning, both add_index and inline t.index paths), and explicit global flag overriding the migration default. * Existing legacy SQL emission specs in `schema_statements_spec.rb` that assert on the implicit constraint output (`should add unique constraint only to the index where it was defined`, `should emit CREATE UNIQUE INDEX and ADD CONSTRAINT for inline t.index unique: true`, `produces the same SQL whether unique index is defined inline or via explicit add_index`) are wrapped in `with_implicit_unique_constraint_enabled` and labeled "(legacy implicit-constraint path)". The helper is a thin wrapper around the global flag, defined in `spec/spec_helper.rb`. * Orthogonal-feature specs in `schema_statements_spec.rb` and `schema_dumper_spec.rb` that needed an index+constraint pair as setup are migrated to `add_unique_constraint`. The remaining remove_index spec that documents the legacy add_index → remove_index pairing keeps the legacy setup via the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 429aa95 commit bcdd815

10 files changed

Lines changed: 275 additions & 63 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: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
# Thread-local key used by V8_1 to opt back into the legacy implicit-
10+
# constraint behavior. Encapsulated here so that the entire
11+
# MigrationCompatibility module — key included — can be removed in
12+
# one piece in Phase 3 of the deprecation (#2702) without leaving
13+
# observable state behind in `Thread.current`.
14+
IMPLICIT_UNIQUE_CONSTRAINT_KEY = :__oracle_enhanced_implicit_unique_constraint
15+
private_constant :IMPLICIT_UNIQUE_CONSTRAINT_KEY
16+
17+
def self.implicit_unique_constraint_enabled?
18+
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] == true
19+
end
20+
21+
def self.with_implicit_unique_constraint_enabled
22+
prev = Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY]
23+
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] = true
24+
yield
25+
ensure
26+
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] = prev
27+
end
28+
29+
# Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation (#2702):
30+
# `Migration[8.2]+` (the current default) treats `add_index unique:
31+
# true` as "create only the unique index" — matching the Rails-core
32+
# PostgreSQL/MySQL/SQLite adapters. `Migration[8.1]` and earlier
33+
# opt back into the pre-Phase-2 behavior of also creating a
34+
# same-named UNIQUE CONSTRAINT, so existing migrations keep
35+
# working unchanged. Callers that need a constraint should call
36+
# `add_unique_constraint :t, :col, name: :n` directly.
37+
module V8_1
38+
def add_index(table_name, column_name, **options)
39+
MigrationCompatibility.with_implicit_unique_constraint_enabled { super }
40+
end
41+
42+
def create_table(table_name, **options, &block)
43+
MigrationCompatibility.with_implicit_unique_constraint_enabled { super }
44+
end
45+
end
46+
end
47+
end
48+
end
49+
end

lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ def add_index(table_name, column_name, **options) # :nodoc:
309309
execute schema_creation.accept(create_index)
310310

311311
index = create_index.index
312-
if needs_unique_constraint?(index.unique, index.columns) && OracleEnhancedAdapter.add_index_unique_creates_constraint
312+
if needs_unique_constraint?(index.unique, index.columns) && implicit_unique_constraint_active?
313313
warn_implicit_unique_constraint_deprecation
314314
execute add_unique_constraint_sql(index.table, index.columns, index.name)
315315
end
@@ -1232,13 +1232,30 @@ def add_unique_constraint_sql(table_name, columns, index_name)
12321232
def add_inline_unique_constraints(table_name, td)
12331233
td.indexes.each do |column_name, index_options|
12341234
next unless needs_unique_constraint?(index_options[:unique], column_name)
1235-
next unless OracleEnhancedAdapter.add_index_unique_creates_constraint
1235+
next unless implicit_unique_constraint_active?
12361236
warn_implicit_unique_constraint_deprecation
12371237
inline_index_name = index_options[:name]&.to_s || index_name(table_name, column: index_column_names(column_name))
12381238
execute add_unique_constraint_sql(table_name, column_name, inline_index_name)
12391239
end
12401240
end
12411241

1242+
# Implicit UNIQUE CONSTRAINT creation for `add_index unique: true`
1243+
# is gated by:
1244+
#
1245+
# * the global flag `add_index_unique_creates_constraint` (explicit
1246+
# user opt-in), OR
1247+
# * `MigrationCompatibility.implicit_unique_constraint_enabled?`,
1248+
# set to true while an `Migration[8.1]` or earlier migration is
1249+
# running (legacy default preserved for old migrations).
1250+
#
1251+
# In Phase 2, the global flag defaults to `false`, so callers
1252+
# outside the V8_1 path (Migration[8.2]+, Schema.define, direct
1253+
# adapter calls) skip the implicit constraint by default.
1254+
def implicit_unique_constraint_active?
1255+
return true if OracleEnhancedAdapter.add_index_unique_creates_constraint
1256+
OracleEnhanced::MigrationCompatibility.implicit_unique_constraint_enabled?
1257+
end
1258+
12421259
def warn_implicit_unique_constraint_deprecation
12431260
OracleEnhanced.deprecator.warn(<<~MSG)
12441261
add_index :col, unique: true creates an implicit named UNIQUE constraint on Oracle,

lib/active_record/connection_adapters/oracle_enhanced_adapter.rb

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
require "active_record/connection_adapters/oracle_enhanced/dbms_output"
5151
require "active_record/connection_adapters/oracle_enhanced/type_metadata"
5252
require "active_record/connection_adapters/oracle_enhanced/structure_dump"
53+
require "active_record/connection_adapters/oracle_enhanced/migration_compatibility"
5354
require "active_record/connection_adapters/oracle_enhanced/structure_dump/dbms_metadata"
5455
require "active_record/connection_adapters/oracle_enhanced/structure_dump/dispatcher"
5556
require "active_record/connection_adapters/oracle_enhanced/lob"
@@ -244,15 +245,22 @@ def ==(other)
244245
# behavior was introduced so that Oracle-specific FK targetability worked
245246
# with Rails-standard <tt>add_index unique: true</tt> migrations, but the
246247
# explicit <tt>add_unique_constraint</tt> DSL is now the recommended path
247-
# for callers who actually need a constraint. Defaults to +true+ for
248-
# backward compatibility; will flip to +false+ in a future release.
248+
# for callers who actually need a constraint.
249249
#
250-
# Set to +false+ in an initializer to opt out of the implicit-constraint
251-
# behavior (and silence the deprecation warning) immediately:
250+
# Phase 2 default: +false+. <tt>Migration[8.2]+</tt> migrations,
251+
# <tt>Schema.define</tt> blocks, and direct adapter calls all default
252+
# to "create the unique index only". <tt>Migration[8.1]</tt> and earlier
253+
# migrations opt back into the legacy implicit-constraint behavior via
254+
# the +MigrationCompatibility+ module so existing migrations keep
255+
# working unchanged.
252256
#
253-
# ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = false
257+
# Set to +true+ explicitly to force the legacy implicit-constraint
258+
# behavior project-wide (e.g. when migrating a long-running multi-DB
259+
# application that depends on it):
260+
#
261+
# ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = true
254262
cattr_accessor :add_index_unique_creates_constraint
255-
self.add_index_unique_creates_constraint = true
263+
self.add_index_unique_creates_constraint = false
256264

257265
##
258266
# :singleton-method:
@@ -567,6 +575,10 @@ def supports_materialized_views?
567575
true
568576
end
569577

578+
def migration_compatibility_module_for(migration_class) # :nodoc:
579+
OracleEnhanced::MigrationCompatibility.module_for(migration_class)
580+
end
581+
570582
def supports_fetch_first_n_rows_and_offset?
571583
return false unless _connection
572584
database_version >= "12"

spec/active_record/connection_adapters/oracle_enhanced/dbms_metadata_structure_dump_spec.rb

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,19 +144,21 @@
144144
# as VISIBLE.
145145
it "emits ALTER INDEX ... INVISIBLE for an INVISIBLE constraint-backed index" do
146146
skip "Not supported in this database version" unless @conn.supports_disabling_indexes?
147-
schema_define do
148-
create_table :test_dbms_meta_inv_uniq, force: true do |t|
149-
t.string :email
147+
with_implicit_unique_constraint_enabled do
148+
schema_define do
149+
create_table :test_dbms_meta_inv_uniq, force: true do |t|
150+
t.string :email
151+
end
152+
add_index :test_dbms_meta_inv_uniq, :email,
153+
unique: true, name: "ix_dbms_meta_inv_uniq", enabled: false
150154
end
151-
add_index :test_dbms_meta_inv_uniq, :email,
152-
unique: true, name: "ix_dbms_meta_inv_uniq", enabled: false
153-
end
154155

155-
dump = @conn.structure_dump
156-
alter_stmt = dump.split("\n\n/\n\n").find do |stmt|
157-
stmt.match?(/\AALTER\s+INDEX\s+"?IX_DBMS_META_INV_UNIQ"?\s+INVISIBLE\s*\z/i)
156+
dump = @conn.structure_dump
157+
alter_stmt = dump.split("\n\n/\n\n").find do |stmt|
158+
stmt.match?(/\AALTER\s+INDEX\s+"?IX_DBMS_META_INV_UNIQ"?\s+INVISIBLE\s*\z/i)
159+
end
160+
expect(alter_stmt).not_to be_nil
158161
end
159-
expect(alter_stmt).not_to be_nil
160162
ensure
161163
schema_define { drop_table :test_dbms_meta_inv_uniq, if_exists: true }
162164
end
@@ -230,7 +232,14 @@
230232
create_table :test_dbms_metadata_posts, force: true do |t|
231233
t.string :email
232234
end
233-
add_index :test_dbms_metadata_posts, :email, unique: true, name: "ix_test_dbms_metadata_email"
235+
end
236+
# Use the legacy implicit-constraint path so this Oracle 12.1+
237+
# DBMS_METADATA path test exercises the "unique index backed by a
238+
# same-name UNIQUE constraint" scenario it was written for.
239+
with_implicit_unique_constraint_enabled do
240+
schema_define do
241+
add_index :test_dbms_metadata_posts, :email, unique: true, name: "ix_test_dbms_metadata_email"
242+
end
234243
end
235244
dump = @conn.structure_dump
236245
stmts = dump.split("\n\n/\n\n")
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe "OracleEnhanced::MigrationCompatibility for add_index unique: true" do
4+
include SchemaSpecHelper
5+
6+
before(:all) do
7+
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
8+
@conn = ActiveRecord::Base.lease_connection
9+
end
10+
11+
before(:each) do
12+
schema_define do
13+
create_table :test_compat_warn, force: true do |t|
14+
t.string :first_name
15+
end
16+
end
17+
end
18+
19+
after(:each) do
20+
schema_define do
21+
drop_table :test_compat_warn, if_exists: true
22+
end
23+
end
24+
25+
def run_migration(migration_class, &body)
26+
klass = Class.new(migration_class) do
27+
define_method(:change, &body)
28+
end
29+
klass.migrate(:up)
30+
end
31+
32+
describe "Migration[8.2] (current default — Phase 2)" do
33+
it "does not create an implicit UNIQUE constraint and does not warn" do
34+
expect {
35+
run_migration(ActiveRecord::Migration[8.2]) do
36+
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_v82
37+
end
38+
}.not_to output(/implicit named UNIQUE constraint/).to_stderr
39+
40+
expect(@conn.indexes(:test_compat_warn).map(&:name)).to include("uniq_v82")
41+
expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).not_to include("uniq_v82")
42+
end
43+
44+
it "does not create the implicit constraint for inline t.index :col, unique: true" do
45+
run_migration(ActiveRecord::Migration[8.2]) do
46+
create_table :test_v82_inline, force: true do |t|
47+
t.string :name
48+
t.index :name, unique: true, name: :uniq_v82_inline
49+
end
50+
end
51+
52+
expect(@conn.indexes(:test_v82_inline).map(&:name)).to include("uniq_v82_inline")
53+
expect(@conn.unique_constraints(:test_v82_inline).map(&:name)).not_to include("uniq_v82_inline")
54+
ensure
55+
schema_define { drop_table :test_v82_inline, if_exists: true }
56+
end
57+
end
58+
59+
describe "Migration[8.1] (legacy — V8_1 module preserves implicit-constraint default)" do
60+
it "creates the implicit UNIQUE constraint and emits the deprecation warning" do
61+
expect {
62+
run_migration(ActiveRecord::Migration[8.1]) do
63+
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_v81
64+
end
65+
}.to output(/implicit named UNIQUE constraint/).to_stderr
66+
67+
expect(@conn.indexes(:test_compat_warn).map(&:name)).to include("uniq_v81")
68+
expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).to include("uniq_v81")
69+
end
70+
71+
it "creates the implicit constraint for inline t.index :col, unique: true" do
72+
expect {
73+
run_migration(ActiveRecord::Migration[8.1]) do
74+
create_table :test_v81_inline, force: true do |t|
75+
t.string :name
76+
t.index :name, unique: true, name: :uniq_v81_inline
77+
end
78+
end
79+
}.to output(/implicit named UNIQUE constraint/).to_stderr
80+
81+
expect(@conn.indexes(:test_v81_inline).map(&:name)).to include("uniq_v81_inline")
82+
expect(@conn.unique_constraints(:test_v81_inline).map(&:name)).to include("uniq_v81_inline")
83+
ensure
84+
schema_define { drop_table :test_v81_inline, if_exists: true }
85+
end
86+
end
87+
88+
describe "explicit global flag overrides Migration[8.2] default" do
89+
around(:each) do |example|
90+
adapter = ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter
91+
previous = adapter.add_index_unique_creates_constraint
92+
example.run
93+
ensure
94+
adapter.add_index_unique_creates_constraint = previous
95+
end
96+
97+
it "creates the implicit constraint when the flag is true even under Migration[8.2]" do
98+
ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = true
99+
100+
expect {
101+
run_migration(ActiveRecord::Migration[8.2]) do
102+
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_flag_override
103+
end
104+
}.to output(/implicit named UNIQUE constraint/).to_stderr
105+
106+
expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).to include("uniq_flag_override")
107+
end
108+
end
109+
end

spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ def drop_test_posts_table
724724

725725
it "does not double-emit t.index unique: true for an index that backs a unique constraint" do
726726
schema_define do
727-
add_index :test_sections, :position, unique: true, name: :uniq_via_idx
727+
add_unique_constraint :test_sections, :position, name: :uniq_via_idx
728728
end
729729
output = dump_table_schema "test_sections"
730730
expect(output).not_to match(/t\.index .*"uniq_via_idx".*unique: true/)

0 commit comments

Comments
 (0)