Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ group :development do
gem "rspec"
gem "rdoc"
gem "rake"
gem "activerecord", github: "rails/rails", branch: "main"
gem "activerecord", github: "yahonda/rails", branch: "per-adapter-migration-compatibility"
gem "ruby-plsql", github: "rsim/ruby-plsql", branch: "master"

platforms :ruby do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

module ActiveRecord
module ConnectionAdapters
module OracleEnhanced
module MigrationCompatibility # :nodoc: all
extend ActiveRecord::Migration::Compatibility::Versioned

# Thread-local key used by V8_1 to opt back into the legacy implicit-
# constraint behavior. Encapsulated here so that the entire
# MigrationCompatibility module — key included — can be removed in
# one piece in Phase 3 of the deprecation (#2702) without leaving
# observable state behind in `Thread.current`.
IMPLICIT_UNIQUE_CONSTRAINT_KEY = :__oracle_enhanced_implicit_unique_constraint
private_constant :IMPLICIT_UNIQUE_CONSTRAINT_KEY

def self.implicit_unique_constraint_enabled?
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] == true
end

def self.with_implicit_unique_constraint_enabled
prev = Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY]
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] = true
yield
ensure
Thread.current[IMPLICIT_UNIQUE_CONSTRAINT_KEY] = prev
end

# Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation (#2702):
# `Migration[8.2]+` (the current default) treats `add_index unique:
# true` as "create only the unique index" — matching the Rails-core
# PostgreSQL/MySQL/SQLite adapters. `Migration[8.1]` and earlier
# opt back into the pre-Phase-2 behavior of also creating a
# same-named UNIQUE CONSTRAINT, so existing migrations keep
# working unchanged. Callers that need a constraint should call
# `add_unique_constraint :t, :col, name: :n` directly.
module V8_1
def add_index(table_name, column_name, **options)
MigrationCompatibility.with_implicit_unique_constraint_enabled { super }
end

def create_table(table_name, **options, &block)
MigrationCompatibility.with_implicit_unique_constraint_enabled { super }
end
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def add_index(table_name, column_name, **options) # :nodoc:
execute schema_creation.accept(create_index)

index = create_index.index
if needs_unique_constraint?(index.unique, index.columns) && OracleEnhancedAdapter.add_index_unique_creates_constraint
if needs_unique_constraint?(index.unique, index.columns) && implicit_unique_constraint_active?
warn_implicit_unique_constraint_deprecation
execute add_unique_constraint_sql(index.table, index.columns, index.name)
end
Expand Down Expand Up @@ -1248,13 +1248,30 @@ def add_unique_constraint_sql(table_name, columns, index_name)
def add_inline_unique_constraints(table_name, td)
td.indexes.each do |column_name, index_options|
next unless needs_unique_constraint?(index_options[:unique], column_name)
next unless OracleEnhancedAdapter.add_index_unique_creates_constraint
next unless implicit_unique_constraint_active?
warn_implicit_unique_constraint_deprecation
inline_index_name = index_options[:name]&.to_s || index_name(table_name, column: index_column_names(column_name))
execute add_unique_constraint_sql(table_name, column_name, inline_index_name)
end
end

# Implicit UNIQUE CONSTRAINT creation for `add_index unique: true`
# is gated by:
#
# * the global flag `add_index_unique_creates_constraint` (explicit
# user opt-in), OR
# * `MigrationCompatibility.implicit_unique_constraint_enabled?`,
# set to true while an `Migration[8.1]` or earlier migration is
# running (legacy default preserved for old migrations).
#
# In Phase 2, the global flag defaults to `false`, so callers
# outside the V8_1 path (Migration[8.2]+, Schema.define, direct
# adapter calls) skip the implicit constraint by default.
def implicit_unique_constraint_active?
return true if OracleEnhancedAdapter.add_index_unique_creates_constraint
OracleEnhanced::MigrationCompatibility.implicit_unique_constraint_enabled?
end

def warn_implicit_unique_constraint_deprecation
OracleEnhanced.deprecator.warn(<<~MSG)
add_index :col, unique: true creates an implicit named UNIQUE constraint on Oracle,
Expand Down
24 changes: 18 additions & 6 deletions lib/active_record/connection_adapters/oracle_enhanced_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
require "active_record/connection_adapters/oracle_enhanced/dbms_output"
require "active_record/connection_adapters/oracle_enhanced/type_metadata"
require "active_record/connection_adapters/oracle_enhanced/structure_dump"
require "active_record/connection_adapters/oracle_enhanced/migration_compatibility"
require "active_record/connection_adapters/oracle_enhanced/structure_dump/dbms_metadata"
require "active_record/connection_adapters/oracle_enhanced/structure_dump/dispatcher"
require "active_record/connection_adapters/oracle_enhanced/lob"
Expand Down Expand Up @@ -244,15 +245,22 @@ def ==(other)
# behavior was introduced so that Oracle-specific FK targetability worked
# with Rails-standard <tt>add_index unique: true</tt> migrations, but the
# explicit <tt>add_unique_constraint</tt> DSL is now the recommended path
# for callers who actually need a constraint. Defaults to +true+ for
# backward compatibility; will flip to +false+ in a future release.
# for callers who actually need a constraint.
#
# Set to +false+ in an initializer to opt out of the implicit-constraint
# behavior (and silence the deprecation warning) immediately:
# Phase 2 default: +false+. <tt>Migration[8.2]+</tt> migrations,
# <tt>Schema.define</tt> blocks, and direct adapter calls all default
# to "create the unique index only". <tt>Migration[8.1]</tt> and earlier
# migrations opt back into the legacy implicit-constraint behavior via
# the +MigrationCompatibility+ module so existing migrations keep
# working unchanged.
#
# ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = false
# Set to +true+ explicitly to force the legacy implicit-constraint
# behavior project-wide (e.g. when migrating a long-running multi-DB
# application that depends on it):
#
# ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = true
cattr_accessor :add_index_unique_creates_constraint
self.add_index_unique_creates_constraint = true
self.add_index_unique_creates_constraint = false

##
# :singleton-method:
Expand Down Expand Up @@ -571,6 +579,10 @@ def supports_materialized_views?
true
end

def migration_compatibility_module_for(migration_class) # :nodoc:
OracleEnhanced::MigrationCompatibility.module_for(migration_class)
end

def supports_fetch_first_n_rows_and_offset?
return false unless _connection
database_version >= "12"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,21 @@
# as VISIBLE.
it "emits ALTER INDEX ... INVISIBLE for an INVISIBLE constraint-backed index" do
skip "Not supported in this database version" unless @conn.supports_disabling_indexes?
schema_define do
create_table :test_dbms_meta_inv_uniq, force: true do |t|
t.string :email
with_implicit_unique_constraint_enabled do
schema_define do
create_table :test_dbms_meta_inv_uniq, force: true do |t|
t.string :email
end
add_index :test_dbms_meta_inv_uniq, :email,
unique: true, name: "ix_dbms_meta_inv_uniq", enabled: false
end
add_index :test_dbms_meta_inv_uniq, :email,
unique: true, name: "ix_dbms_meta_inv_uniq", enabled: false
end

dump = @conn.structure_dump
alter_stmt = dump.split("\n\n/\n\n").find do |stmt|
stmt.match?(/\AALTER\s+INDEX\s+"?IX_DBMS_META_INV_UNIQ"?\s+INVISIBLE\s*\z/i)
dump = @conn.structure_dump
alter_stmt = dump.split("\n\n/\n\n").find do |stmt|
stmt.match?(/\AALTER\s+INDEX\s+"?IX_DBMS_META_INV_UNIQ"?\s+INVISIBLE\s*\z/i)
end
expect(alter_stmt).not_to be_nil
end
expect(alter_stmt).not_to be_nil
ensure
schema_define { drop_table :test_dbms_meta_inv_uniq, if_exists: true }
end
Expand Down Expand Up @@ -230,7 +232,14 @@
create_table :test_dbms_metadata_posts, force: true do |t|
t.string :email
end
add_index :test_dbms_metadata_posts, :email, unique: true, name: "ix_test_dbms_metadata_email"
end
# Use the legacy implicit-constraint path so this Oracle 12.1+
# DBMS_METADATA path test exercises the "unique index backed by a
# same-name UNIQUE constraint" scenario it was written for.
with_implicit_unique_constraint_enabled do
schema_define do
add_index :test_dbms_metadata_posts, :email, unique: true, name: "ix_test_dbms_metadata_email"
end
end
dump = @conn.structure_dump
stmts = dump.split("\n\n/\n\n")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# frozen_string_literal: true

RSpec.describe "OracleEnhanced::MigrationCompatibility for add_index unique: true" do
include SchemaSpecHelper

before(:all) do
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
@conn = ActiveRecord::Base.lease_connection
end

before(:each) do
schema_define do
create_table :test_compat_warn, force: true do |t|
t.string :first_name
end
end
end

after(:each) do
schema_define do
drop_table :test_compat_warn, if_exists: true
end
end

def run_migration(migration_class, &body)
klass = Class.new(migration_class) do
define_method(:change, &body)
end
klass.migrate(:up)
end

describe "Migration[8.2] (current default — Phase 2)" do
it "does not create an implicit UNIQUE constraint and does not warn" do
expect {
run_migration(ActiveRecord::Migration[8.2]) do
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_v82
end
}.not_to output(/implicit named UNIQUE constraint/).to_stderr

expect(@conn.indexes(:test_compat_warn).map(&:name)).to include("uniq_v82")
expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).not_to include("uniq_v82")
end

it "does not create the implicit constraint for inline t.index :col, unique: true" do
run_migration(ActiveRecord::Migration[8.2]) do
create_table :test_v82_inline, force: true do |t|
t.string :name
t.index :name, unique: true, name: :uniq_v82_inline
end
end

expect(@conn.indexes(:test_v82_inline).map(&:name)).to include("uniq_v82_inline")
expect(@conn.unique_constraints(:test_v82_inline).map(&:name)).not_to include("uniq_v82_inline")
ensure
schema_define { drop_table :test_v82_inline, if_exists: true }
end
end

describe "Migration[8.1] (legacy — V8_1 module preserves implicit-constraint default)" do
it "creates the implicit UNIQUE constraint and emits the deprecation warning" do
expect {
run_migration(ActiveRecord::Migration[8.1]) do
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_v81
end
}.to output(/implicit named UNIQUE constraint/).to_stderr

expect(@conn.indexes(:test_compat_warn).map(&:name)).to include("uniq_v81")
expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).to include("uniq_v81")
end

it "creates the implicit constraint for inline t.index :col, unique: true" do
expect {
run_migration(ActiveRecord::Migration[8.1]) do
create_table :test_v81_inline, force: true do |t|
t.string :name
t.index :name, unique: true, name: :uniq_v81_inline
end
end
}.to output(/implicit named UNIQUE constraint/).to_stderr

expect(@conn.indexes(:test_v81_inline).map(&:name)).to include("uniq_v81_inline")
expect(@conn.unique_constraints(:test_v81_inline).map(&:name)).to include("uniq_v81_inline")
ensure
schema_define { drop_table :test_v81_inline, if_exists: true }
end
end

describe "explicit global flag overrides Migration[8.2] default" do
around(:each) do |example|
adapter = ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter
previous = adapter.add_index_unique_creates_constraint
example.run
ensure
adapter.add_index_unique_creates_constraint = previous
end

it "creates the implicit constraint when the flag is true even under Migration[8.2]" do
ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = true

expect {
run_migration(ActiveRecord::Migration[8.2]) do
add_index :test_compat_warn, :first_name, unique: true, name: :uniq_flag_override
end
}.to output(/implicit named UNIQUE constraint/).to_stderr

expect(@conn.unique_constraints(:test_compat_warn).map(&:name)).to include("uniq_flag_override")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ def drop_test_posts_table

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