Phase 2: flip add_index unique: true default in Migration[8.2]+ [DRAFT — depends on Rails per-adapter-migration-compatibility]#2710
Draft
yahonda wants to merge 1 commit into
Conversation
fd16113 to
9c33a57
Compare
This was referenced May 7, 2026
9c33a57 to
13dde32
Compare
13dde32 to
cca3385
Compare
Closes rsim#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>
cca3385 to
bcdd815
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2702 (Phase 2).
This PR is a draft because it depends on the unmerged Rails extension point
AbstractAdapter#migration_compatibility_module_for(migration_class)plus theMigration::Compatibility::Versionedconvention from yahonda/railsper-adapter-migration-compatibility— same dependency as #2596. Once that lands onrails/rails#main:Gemfileback torails/rails#mainIn Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation, the global default flips from "implicit constraint + warning" (Phase 1, shipped in #2709) 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 aMigrationCompatibility::V8_1module so they keep working unchanged.Behavior matrix
master)Migration[8.2](Current) —add_index unique: trueMigration[8.1]and earlier —add_index unique: trueSchema.define { add_index unique: true }connection.add_index unique: trueOracleEnhancedAdapter.add_index_unique_creates_constraint = true(explicit) — any callerChanges
Production
OracleEnhancedAdapter.add_index_unique_creates_constraintdefault flips fromtruetofalse. Documentation updated to reflect the Phase 2 default and the explicit opt-in for legacy behavior.lib/active_record/connection_adapters/oracle_enhanced/migration_compatibility.rb:MigrationCompatibility::V8_1module wrapsadd_indexandcreate_tablesoMigration[8.1]and earlier migrations re-enable the implicit-constraint path.:__oracle_enhanced_implicit_unique_constraint) is encapsulated as aprivate_constantofMigrationCompatibility, withwith_implicit_unique_constraint_enabled/implicit_unique_constraint_enabled?accessors. Phase 3 deletes the whole module — key, accessors, and V8_1 — in one piece without leaving observable state behind inThread.current.OracleEnhancedAdapter#migration_compatibility_module_for(migration_class)returnsMigrationCompatibility.module_for(migration_class), mirroring the PG / MySQL / SQLite adapters inper-adapter-migration-compatibility.schema_statements.rbintroduces animplicit_unique_constraint_active?private helper that ORs the global flag with the migration-version thread-local, used by bothadd_indexandadd_inline_unique_constraints.Specs
migration_compatibility_spec.rbcovering:add_indexand inlinet.indexpathsexpect { ... }.to output(...).to_stderr, both pathsadd_index_unique_creates_constraint = true— overrides the Migration[8.2] defaultshould 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, the DBMS_METADATA path'sdoes not emit a standalone CREATE INDEX for a UNIQUE constraint's backing index, andemits ALTER INDEX ... INVISIBLE for an INVISIBLE constraint-backed index) → wrapped in a newwith_implicit_unique_constraint_enabledhelper and labeled "(legacy implicit-constraint path)". These specs intentionally exercise the legacy code path; the helper makes that intent explicit.structure_dump'sappends NOVALIDATE for foreign keys added with validate: false) that just needed an index + constraint pair as setup are migrated toadd_unique_constraint. Forward-compatible: Phase 3 doesn't need to touch them.drops both the index and the same-name implicit constraint when add_index unique: true created themspec forremove_indexkeeps the legacyadd_index unique: truesetup via the helper, because that pairing is precisely what's being tested. (Per "let Oracle handle constraint-managed indexes" — this scenario only happens for the legacy path where the index is created explicitly viaCREATE UNIQUE INDEX, not auto-created behind a constraint.)spec_helper.rbaddsImplicitUniqueConstraintHelper#with_implicit_unique_constraint_enabled, included globally so any spec can opt-in to legacy.Out of scope
remove_indexagainst an index that was auto-created behind anadd_unique_constraintstill raisesORA-01418because Oracle's automatic cleanup drops the index together with the constraint, thenremove_indextries to drop the (already-gone) index. Per the design principle "let Oracle handle the index it auto-creates behind a constraint", the recovery path for that case isremove_unique_constraint :t, name: :n, notremove_index. Worth a friendly error in a follow-up issue if it bites users in practice.Test plan
bundle exec rspec spec/active_record/connection_adapters/oracle_enhanced/migration_compatibility_spec.rb(5 examples)bundle exec rspec spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb -e "implicit constraint deprecation"(6 examples) — Phase 1 specs continue to pass; the explicit-flag-true ones still test the legacy path nowCI=1 bundle exec rspec spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb spec/active_record/connection_adapters/oracle_enhanced/structure_dump_spec.rb spec/active_record/connection_adapters/oracle_enhanced/migration_compatibility_spec.rb— 226 examples, 0 failures, 2 pre-existing pendingbundle exec rubocop— cleanGemfileback torails/rails#mainonceper-adapter-migration-compatibilitylandsRelated
per-adapter-migration-compatibility(same as Phase 2: Auto-enable identity primary keys in Migration[8.2]+ (depends on Rails per-adapter-migration-compatibility) [DRAFT] #2596)🤖 Generated with Claude Code