Phase 2: Auto-enable identity primary keys in Migration[8.2]+ (depends on Rails per-adapter-migration-compatibility) [DRAFT]#2596
Open
yahonda wants to merge 1 commit into
Conversation
eb3fd69 to
a94a652
Compare
a94a652 to
bdbf838
Compare
3 tasks
529545a to
4d7a6c9
Compare
3 tasks
4d7a6c9 to
7741715
Compare
This was referenced May 5, 2026
7741715 to
2b74926
Compare
Phase 2 of rsim#2579: build on the explicit `identity:` option from Phase 1 and switch the default for migrations declared at `ActiveRecord::Migration[8.2]` or later. Older migration versions, `Schema.define`, and direct `connection.create_table` keep the existing sequence + trigger behavior, so existing applications see no change unless they explicitly adopt `Migration[8.2]`. Wired through the new per-adapter extension point `AbstractAdapter#migration_compatibility_module_for(migration_class)` from yahonda/rails `per-adapter-migration-compatibility` (not yet on rails master). The Gemfile temporarily points at that branch; once the Rails commit lands on `rails/rails#main` the Gemfile reference can be reverted. Architecture: two compat modules layered via Rails' versioned dispatch. `V8_2` forces `identity: true` for the current default; `V8_1` forces `identity: false` to preserve legacy behavior on older migrations. The adapter's underlying `create_table` honors `options[:identity]` exactly as given and does no auto-injection, so callers outside the migration compat chain (`Schema.define`, direct `connection.create_table`) keep sequence-backed PKs by default. `db/schema.rb` roundtrips remain safe because the schema dumper now emits `identity: true` / `identity: false` explicitly for every primary-key table. `V8_2` also treats explicit Oracle sequence options as opt-out: when `sequence_name:` or `sequence_start_value:` is given, the requested sequence is created and the PK stays sequence-backed. `V8_2` pre-checks `connection.supports_identity_columns?` so `Migration[8.2]+` on Oracle <12.1 silently falls back to the legacy sequence path instead of triggering the Phase 1 ArgumentError. Per-table opt-out remains via `identity: false`; per-migration opt-out via `Migration[8.1]` or earlier. `rename_table` and `drop_table` are now identity-aware: they no longer rename or drop `<table>_seq` for tables whose primary key is an Oracle identity column. Previously, an unrelated application-owned sequence sharing that name would be silently mutated or removed when the table was renamed/dropped.
2b74926 to
ba8c016
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
Phase 2 of splitting #2579: build on Phase 1's explicit
identity:option (now merged) and flip the default for
create_tabletoidentity: trueon Oracle 12.1+, scoped to migrations declared atMigration[8.2]or later.Migration[8.1]and earlier,Schema.define,and direct
connection.create_tablekeep the existing sequence + triggerbehavior, so applications see no change unless they explicitly adopt
Migration[8.2].This PR is a draft because it depends on the unmerged Rails extension
point
AbstractAdapter#migration_compatibility_module_for(migration_class)plus the
Migration::Compatibility::Versionedconvention fromyahonda/rails
per-adapter-migration-compatibility.Once that lands on
rails/rails#main:Gemfileback torails/rails#mainBehavior matrix
identity:argMigration[8.2]+Migration[8.2]+supports_identity_columns?)Migration[8.2]+sequence_name:/sequence_start_value:Migration[8.2]+trueMigration[8.2]+trueArgumentError(Phase 1)Migration[8.2]+falseMigration[8.1]or olderMigration[8.1]or oldertrueMigration[8.1]or oldertrueArgumentErrorMigration[8.1]or olderfalseSchema.define/ directconnection.create_tableSchema.define/ directconnection.create_tabletrueSchema.define/ directconnection.create_tablefalseImplementation notes
OracleEnhanced::MigrationCompatibilityregisters two compat modulesvia Rails'
Migration::Compatibility::Versioned:V8_2forcesoptions[:identity] = truewhen the option is omitted,no
sequence_name:/sequence_start_value:is given,id: :primary_keyis in effect (default), the PK is not composite,and the server supports identity columns. This is the new flip,
scoped to migrations at
Migration[8.2]+.V8_1forcesoptions[:identity] = falsefor migrations atMigration[8.1]or earlier. Because Rails' versioned dispatchincludes both modules for older migrations and V8_1 sets the
explicit option first, V8_2 sees
options.key?(:identity)andleaves it alone — older migrations keep sequence-backed behavior.
OracleEnhancedSchemaStatements#create_tablehonorsoptions[:identity]exactly as given and does no auto-injectionof its own. Callers that bypass migration compat (
Schema.define,direct
connection.create_table) therefore keep the existingsequence-backed default unless they pass
identity: trueexplicitly.OracleEnhancedSchemaStatements#rename_tableand#drop_tablearenow identity-aware: they no longer rename or drop
<table>_seqfortables whose primary key is an Oracle identity column. Previously,
an unrelated application-owned sequence sharing that name would be
silently mutated or removed.
full_dropalready had this protectionvia its
ISEQ$$_filter; this brings the table-level DDL paths inline.
OracleEnhancedSchemaDumperemitsidentity: true/identity: falseexplicitly for every primary-key table when the connected server
supports identity columns, so
db/schema.rbroundtrips remainunambiguous regardless of which loader runs them.
OracleEnhancedDatabaseStatements#empty_insert_statement_valueisalready in master from Implement empty_insert_statement_value for identity primary keys #2609 — this PR depends on that change so
identity tables can satisfy
Model.create!with no attributes.Tests
migration_compatibility_spec.rbcases exercise the V8_2 / V8_1layering, the sequence-option opt-out, and the
Schema.define/direct-call path that preserves sequence-backed PKs by default.
identity_primary_key_spec.rbcases cover identity-awarerename_tableanddrop_tableagainst an unrelated app-owned<table>_seq, plus the identity counterparts to the existingsequence-prerequisite tests (
pk_and_sequence_for,NEXTVAL-freeinsert,
full_dropfilter, long-name rename).Test plan
bundle exec rubocop— cleanrails/rails#mainonceper-adapter-migration-compatibilitylandsRelated
identity:option) — mergedempty_insert_statement_value) — mergedper-adapter-migration-compatibility🤖 Generated with Claude Code