v3: Phase 2 — flip add_index unique: true default in Migration[8.2]+#2814
Open
yahonda wants to merge 3 commits into
Open
v3: Phase 2 — flip add_index unique: true default in Migration[8.2]+#2814yahonda wants to merge 3 commits into
yahonda wants to merge 3 commits into
Conversation
Adopt identity primary keys as the default for `Migration[8.2]` and
later when the server supports them (Oracle Database 12.1+); preserve
sequence-backed primary keys for `Migration[8.1]` and earlier, for
`Schema.define`, and for direct `connection.create_table` calls so
existing schemas and dumps replay unchanged.
* New `OracleEnhanced::CompatibilityStrategy` (`extend Base::Resolver`)
routes the per-version behavior via Rails'
`compatibility_strategy_for(migration_class)` hook:
* `V8_2` forces `options[:identity] = true` when the option is
omitted, no sequence options exist, the connection supports
identity columns, and the caller is not `ActiveRecord::Schema`
(so `Schema.define` keeps the sequence default).
* `V8_1` forces `options[:identity] = false` to maintain legacy
sequence behavior on older migrations.
* `rename_table` and `drop_table` become identity-aware: they skip the
paired `<table>_seq` rename/drop when the table's primary key is an
Oracle identity column, so unrelated application-owned sequences with
matching names are left untouched.
* The schema dumper emits `identity: false` for sequence-backed primary
keys on identity-capable servers, so a dump produced under the new
default reloads to a sequence-backed table when desired.
* Gemfile points activerecord at yahonda/rails
per-adapter-migration-compatibility-v3 for the framework dispatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation (rsim#2702). For `Migration[8.2]` and later, `add_index :col, unique: true` now creates only the unique index (matching Rails-core PostgreSQL/MySQL/SQLite). `Migration[8.1]` and earlier keep the legacy behavior of also creating a same-named UNIQUE CONSTRAINT, so existing migrations replay unchanged. Callers that need a constraint on Migration[8.2]+ should call `add_unique_constraint :t, :col, name: :n` directly. * `OracleEnhanced::CompatibilityStrategy::V8_1` now sets a private `_implicit_unique_constraint: true` flag in `create_table` and `add_index` (in addition to the identity-default backstop). The adapter consumes and deletes the flag before further processing, so it never reaches DDL emission. * New `implicit_unique_constraint_active?` helper in the adapter ORs the global `add_index_unique_creates_constraint` flag with the per-call `_implicit_unique_constraint` flag, keeping the single decision point readable. * `OracleEnhancedAdapter.add_index_unique_creates_constraint` default flips from `true` to `false`. The flag still exists as an explicit opt-in for projects that want the legacy implicit-constraint behavior project-wide. * Specs that exercise the legacy path use the new `with_implicit_unique_constraint_enabled` helper in `spec_helper.rb`, which toggles the global flag around a block. The helper is loaded globally and can be removed in Phase 3 alongside the flag. * `migration_compatibility_spec.rb` grows a second top-level describe block covering `add_index unique: true` under both Migration[8.2] (no implicit constraint, no warning) and Migration[8.1] (implicit constraint, deprecation warning), plus the inline `t.index` cases and an explicit-flag override case. Depends on the Migration[8.2]+ identity primary key default (yahonda/oracle-enhanced#TBD), which introduces the v3 CompatibilityStrategy plumbing this PR extends. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rubocop's Layout/EmptyLines flagged the double blank line introduced when the unique-constraint describe block was appended to the identity describe block in the same file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
v3 redo of #2710 (Phase 2 of the implicit-UNIQUE-CONSTRAINT deprecation #2702) against the new
Rails::Migration::CompatibilityStrategyextension point.Behavior
Migration[8.2]and later treatadd_index :col, unique: trueas "create only the unique index" — matching Rails-core PostgreSQL/MySQL/SQLite.Migration[8.1]and earlier keep the legacy behavior of also creating a same-named UNIQUE CONSTRAINT, so existing migrations replay unchanged. Callers that need a constraint on Migration[8.2]+ should calladd_unique_constraint :t, :col, name: :ndirectly.Implementation
OracleEnhanced::CompatibilityStrategy::V8_1now sets a private_implicit_unique_constraint: trueflag increate_tableandadd_index(in addition to the identity-default backstop from the previous PR). The adapter consumes and deletes the flag before further processing, so it never reaches DDL emission.implicit_unique_constraint_active?adapter helper ORs the globaladd_index_unique_creates_constraintflag with the per-call_implicit_unique_constraintflag.OracleEnhancedAdapter.add_index_unique_creates_constraintdefault flips fromtruetofalse. The flag remains as an explicit opt-in for projects that want the legacy behavior project-wide.with_implicit_unique_constraint_enabledinspec_helper.rb, which toggles the global flag around a block.migration_compatibility_spec.rbgains a second top-level describe block covering both Migration[8.2] (no implicit constraint, no warning) and Migration[8.1] (implicit constraint + deprecation warning), inlinet.indexcases, and an explicit-flag override case.Compared to #2710 Phase 2
include-based modules. Notably, this PR uses an options-flag signal (_implicit_unique_constraint) rather than the thread-localwith_implicit_unique_constraint_enabledcarried by Phase 2: flip add_index unique: true default in Migration[8.2]+ [DRAFT — depends on Rails per-adapter-migration-compatibility] #2710 Phase 2. Migrations are single-threaded, the strategy is the wrapper itself, and the flag is consumed/deleted by the adapter before DDL emission — so thread-local state is not needed and the explicit options flag keeps the data flow visible.Tests
All exercised against Oracle Free 23ai:
migration_compatibility_spec.rb(combined identity + unique),identity_primary_key_spec.rb,schema_dumper_spec.rb,schema_statements_spec.rb,structure_dump_spec.rball pass.Depends on
V8_1#create_tableidentity backstop). Merge that first.compatibility_strategy_forandMigration::CompatibilityStrategy(currentlyyahonda/railsbranchper-adapter-migration-compatibility-v3).