Skip to content
Open
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-v7"
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,68 @@
# frozen_string_literal: true

module ActiveRecord
module ConnectionAdapters
module OracleEnhanced
module CompatibilityBehavior # :nodoc: all
Base = ActiveRecord::Migration::CompatibilityBehavior
extend Base::Resolver

# A behavior applies to migrations declaring its own version and older:
# Migration[8.2]+ resolves to V8_2, Migration[8.1] and earlier to V8_1.
class V8_2 < Base; end

# Migration[8.1] and earlier keep the pre-8.2 implicit-UNIQUE-CONSTRAINT
# behavior for `add_index unique: true`, so existing migrations replay
# unchanged. The +_implicit_unique_constraint+ flag is consumed and
# deleted by the adapter before any further processing. Callers that
# need a constraint on Migration[8.2]+ should call
# `add_unique_constraint :t, :col, name: :n` directly.
class V8_1 < V8_2
def create_table(table_name, **options)
options[:_implicit_unique_constraint] = true
super
end

def add_index(table_name, column_name, **options)
options[:_implicit_unique_constraint] = true
super
end

def add_reference(table_name, *ref_names, **options)
index = options[:index]
if index.is_a?(Hash) && index[:unique]
options[:index] = index.merge(_implicit_unique_constraint: true)
end
super
end
alias :add_belongs_to :add_reference

def create_join_table(table_1, table_2, **options)
options[:_implicit_unique_constraint] = true
super
end

# Prepended onto the change_table receiver by the framework's
# compatible_table_definition. Inject only there: the create_table
# path is covered by the table-level flag, and Rails validates
# per-index option keys while creating the table.
module TableDefinition
def index(column_name, **options)
options[:_implicit_unique_constraint] = true if ActiveRecord::ConnectionAdapters::Table === self
super
end

def references(*args, **options)
index = options[:index]
if ActiveRecord::ConnectionAdapters::Table === self && index.is_a?(Hash) && index[:unique]
options[:index] = index.merge(_implicit_unique_constraint: true)
end
super
end
alias :belongs_to :references
end
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "openssl"
require "active_record/connection_adapters/oracle_enhanced/compatibility_behavior"

module ActiveRecord
module ConnectionAdapters
Expand All @@ -10,6 +11,10 @@ module SchemaStatements
#
# see: abstract/schema_statements.rb

def compatibility_behavior_for(migration_class) # :nodoc:
OracleEnhanced::CompatibilityBehavior.for(migration_class)
end

def tables # :nodoc:
select_values(<<~SQL.squish, "SCHEMA")
SELECT
Expand Down Expand Up @@ -231,6 +236,8 @@ def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **o
raise ArgumentError, "Options `:force` and `:if_not_exists` cannot be used simultaneously."
end

implicit_unique_constraint = options.delete(:_implicit_unique_constraint)

identity = options[:identity]
validate_identity_options!(identity, id, primary_key)
validate_primary_key_trigger_options!(options[:primary_key_trigger], identity, id, primary_key)
Expand All @@ -250,7 +257,7 @@ def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **o
yield td if block_given?
end

add_inline_unique_constraints(table_name, captured_td)
add_inline_unique_constraints(table_name, captured_td, implicit_unique_constraint)

create_pk_sequence(table_name, options) if should_create_sequence?(captured_td, id, identity)
create_pk_trigger(table_name, primary_key, options) if options[:primary_key_trigger]
Expand All @@ -273,6 +280,8 @@ def rename_table(table_name, new_name, **options) # :nodoc:
end

def drop_table(*table_names, **options) # :nodoc:
# Arrives via CommandRecorder inversions of V8_1-flagged create_table calls.
options.delete(:_implicit_unique_constraint)
# :sequence_name names a single sequence, so it cannot unambiguously
# apply across multiple tables; honor it only for single-table drops.
custom_sequence_name = table_names.size == 1 ? options[:sequence_name] : nil
Expand Down Expand Up @@ -303,13 +312,14 @@ def drop_if_exists(object_type, name, if_exists: true, cascade_constraints: fals
end

def add_index(table_name, column_name, **options) # :nodoc:
implicit_unique_constraint = options.delete(:_implicit_unique_constraint)
create_index = build_create_index_definition(table_name, column_name, **options)
return unless create_index

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?(implicit_unique_constraint)
warn_implicit_unique_constraint_deprecation
execute add_unique_constraint_sql(index.table, index.columns, index.name)
end
Expand Down Expand Up @@ -355,6 +365,8 @@ def add_index_options(table_name, column_name, name: nil, if_not_exists: false,
# Remove the given index from the table.
# Gives warning if index does not exist
def remove_index(table_name, column_name = nil, **options) # :nodoc:
# Arrives via CommandRecorder inversions of V8_1-flagged add_index calls.
options.delete(:_implicit_unique_constraint)
return if options[:if_exists] && !index_exists?(table_name, column_name, **options)

index_name = index_name_for_remove(table_name, column_name, options).to_s
Expand Down Expand Up @@ -1245,38 +1257,58 @@ def add_unique_constraint_sql(table_name, columns, index_name)
"ALTER TABLE #{quote_table_name(table_name)} ADD CONSTRAINT #{quote_column_name(index_name)} UNIQUE (#{quoted_cols}) USING INDEX #{quote_column_name(index_name)}"
end

def add_inline_unique_constraints(table_name, td)
def add_inline_unique_constraints(table_name, td, implicit_unique_constraint = false)
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?(implicit_unique_constraint)
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
# * the `_implicit_unique_constraint` flag set by the V8_1
# compatibility behavior while a Migration[8.1] or earlier
# migration is running (pre-Phase 2 default preserved).
#
# 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?(flag)
OracleEnhancedAdapter.add_index_unique_creates_constraint || flag == true
end

def warn_implicit_unique_constraint_deprecation
OracleEnhanced.deprecator.warn(<<~MSG)
add_index :col, unique: true creates an implicit named UNIQUE constraint on Oracle,
in addition to the unique index. This implicit-constraint behavior will be removed
in a future oracle-enhanced release.

To silence this warning, set the flag in an initializer:

ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = false
in addition to the unique index, because this migration replays the pre-8.2
behavior: it declares ActiveRecord::Migration[8.1] or earlier, or
`add_index_unique_creates_constraint` is enabled globally. This
implicit-constraint behavior will be removed in a future oracle-enhanced release.

After setting the flag, choose the path that matches your intent:
Choose the path that matches your intent:

* Unique INDEX only (typical when foreign keys reference primary keys):
no migration changes needed — add_index :col, unique: true keeps creating
the unique index, just without the extra UNIQUE CONSTRAINT.
declare the migration as ActiveRecord::Migration[8.2] or later —
add_index :col, unique: true then creates only the unique index.

* Unique INDEX + UNIQUE CONSTRAINT (needed when this column is a non-PK
foreign-key target, which Oracle only allows against named constraints):
use add_unique_constraint instead — it creates both the constraint and
its backing unique index in one call, e.g.

add_unique_constraint :sections, :position, name: :uniq_position

* If this warning comes from the global flag, remove

ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = true

from your initializer.
MSG
end

Expand Down
19 changes: 13 additions & 6 deletions lib/active_record/connection_adapters/oracle_enhanced_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,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 pre-8.2 implicit-constraint behavior via
# the +CompatibilityBehavior+ so existing migrations keep working
# unchanged.
#
# ActiveRecord::ConnectionAdapters::OracleEnhancedAdapter.add_index_unique_creates_constraint = false
# Set to +true+ explicitly to force the pre-8.2 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
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 pre-8.2 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 Expand Up @@ -262,7 +271,13 @@
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
# The pre-8.2 implicit-constraint path recreates the multi-statement
# `GET_DDL` CLOB shape this regression test documents.
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

temp_file = Tempfile.create(["oracle_enhanced_roundtrip", ".sql"]).path
Expand Down
Loading
Loading