Skip to content

Add composite primary key support#2693

Merged
yahonda merged 1 commit into
rsim:masterfrom
yahonda:composite-pk-2648
May 5, 2026
Merged

Add composite primary key support#2693
yahonda merged 1 commit into
rsim:masterfrom
yahonda:composite-pk-2648

Conversation

@yahonda
Copy link
Copy Markdown
Collaborator

@yahonda yahonda commented May 5, 2026

Summary

Adds full composite primary key support to the Oracle Enhanced adapter, tracking the native Active Record CPK feature added in Rails 7.1 (rails/rails#48541). Closes #2648 and supersedes the partial fix in #2530.

Until now, oracle-enhanced treated composite PKs as unsupported:

  • pk_and_sequence_for emitted a runtime warning (visible in every test run that touched a composite-PK table such as test_authors_test_books) and returned nil.
  • primary_key returned nil for composite-PK tables.
  • The insert / RETURNING path was single-column-only.
  • empty_insert_statement_value raised NotImplementedError for an Array primary key.

Adapter changes

  • pk_and_sequence_for — drop the "Composite primary key is ignored" warning. The method still returns nil for composite PKs (a single sequence cannot model a composite key); callers fall back to primary_keys.
  • primary_key — removed; Rails' abstract default (pk = pk.first unless pk.size > 1) already returns an Array for composite PKs and a String / nil for single-column / no-PK tables.
  • has_primary_key? — uses primary_keys directly so it returns true for composite-PK tables.
  • prefetch_primary_key_from_dictionary — short-circuits composite PKs to false using a cardinality check (pks.size > 1), not composite_primary_key?(pks) — the predicate is Array?-based and primary_keys always returns an Array (including ["id"] for single-column PKs), so an Array? check would have wrongly disabled prefetch for every PK table when the schema cache is cold. The schema-cache fast path keeps using the existing composite_primary_key?(pks) predicate, where schema_cache.primary_keys returns a String for single-column PKs.
  • empty_insert_statement_value — accepts an Array primary key and produces a multi-column (c1, c2, ...) VALUES (DEFAULT, DEFAULT, ...) form.
  • columns_for_returning_clause — accepts an Array pk so multi-column RETURNING is generated. The "all-defaults" rejection-skip regex is widened from /VALUES \(DEFAULT\)/ to /VALUES \(DEFAULT(?:, DEFAULT)*\)/ so composite PKs whose columns all default still get RETURNING for the database-generated values rather than being dropped by the column-list rejection logic.
  • write_lobs — gets a TODO documenting that its scalar-PK assumption breaks for CPK + LOB models. Out of scope for this PR; tracked separately.

Validations stay as-is: identity: true and primary_key_trigger: true combined with a composite primary key still raise ArgumentError.

Tests

  • New spec/.../composite_primary_key_spec.rb ports the adapter-relevant cases from Rails' own activerecord/test/cases/primary_keys_test.rb so they run against Oracle. Coverage:
    • introspection (primary_keys, primary_key, has_primary_key?, pk_and_sequence_for returning nil and emitting no warning)
    • schema cache caching and invalidation of the composite PK column array
    • model contract (composite_primary_key?, id= array assignment, id= with non-array raising TypeError, id_was, id?, to_key, primary_key_values_present?)
    • schema dump round-trip (in-order and out-of-order composite PKs)
    • prefetch_primary_key? — false for composite, true for single PK, including a cold-cache regression that exercises the dictionary fallback
    • RETURNING — both the "user supplies all PK columns" and the "all PK columns defaulted" path on a CPK table with column defaults
    • find / find with multiple keys (preserves request order) / wrapped-array find / class-level destroy / predicate-builder where-by-PK-columns
    • mixed-type CPK (String + Integer) round-trip and find/destroy
  • identity_primary_key_spec.rb's empty_insert_statement_value composite assertion is updated to expect the new SQL fragment instead of NotImplementedError.

Test plan

  • bundle exec rspec spec/active_record/connection_adapters/oracle_enhanced/composite_primary_key_spec.rb — all examples pass
  • Full local suite on Oracle 23ai (FREEPDB1) — 804 examples, 0 failures, 12 pending (all pre-existing skips)
  • Local suite output no longer contains WARNING: Active Record does not support composite primary key.
  • bundle exec rubocop — clean
  • Fork CI on Ruby 3.3 / 3.4 / 4.0 / JRuby 10.1.0.0 + RuboCop — all green on the squashed head
  • Upstream CI on the full matrix (does not auto-run on fork PRs; needs maintainer approval)

Active Record gained native composite-primary-key support in Rails 7.1
(rails/rails#48541), but oracle-enhanced still treated composite PKs
as unsupported: pk_and_sequence_for emitted a runtime warning and
returned nil, primary_key returned nil for composite-PK tables, the
insert / RETURNING path was single-column-only, and
empty_insert_statement_value raised NotImplementedError for an Array
primary key.

Adapter changes:

- Drop the runtime "Composite primary key is ignored" warning from
  pk_and_sequence_for. The method still returns nil for composite PKs
  (a single sequence does not model a composite key); callers fall
  back to primary_keys.
- Remove the primary_key override; Rails' abstract default
  (`pk = pk.first unless pk.size > 1`) already returns an Array for
  composite PKs and a String / nil for single-column / no-PK tables.
- has_primary_key? now uses primary_keys directly so it returns true
  for composite-PK tables.
- prefetch_primary_key_from_dictionary short-circuits composite PKs
  to false (cardinality check, not Array? — the adapter's
  primary_keys always returns an Array, including ["id"] for
  single-column PKs). The schema-cache helper already handled
  composite via composite_primary_key?(pks). Composite PKs cannot be
  prefetched from a single sequence and instead go through the
  RETURNING-driven insert path.
- empty_insert_statement_value accepts an Array primary key and
  produces a multi-column "(c1, c2, ...) VALUES (DEFAULT, DEFAULT, ...)"
  form for the all-defaults case.
- columns_for_returning_clause accepts an Array pk so multi-column
  RETURNING is generated. The "all-defaults" rejection-skip regex is
  widened from /VALUES \(DEFAULT\)/ to
  /VALUES \(DEFAULT(?:, DEFAULT)*\)/ so composite PKs whose columns
  all default still get RETURNING for the database-generated values
  rather than being dropped by the column-list rejection logic.

write_lobs gets a TODO documenting that its scalar-PK assumption
breaks for CPK + LOB models. Out of scope for this change; tracked
separately.

Validations stay as-is: identity: true and primary_key_trigger: true
combined with a composite primary key still raise ArgumentError.

Tests:

- New spec/.../composite_primary_key_spec.rb ports the
  adapter-relevant cases from Rails' own
  activerecord/test/cases/primary_keys_test.rb so they run against
  Oracle. Coverage:
  - introspection (primary_keys, primary_key, has_primary_key?,
    pk_and_sequence_for returning nil and emitting no warning);
  - schema cache caching and invalidation of the composite PK column
    array;
  - model contract (composite_primary_key?, id= array assignment,
    id= with non-array raising TypeError, id_was, id?, to_key,
    primary_key_values_present?);
  - schema dump round-trip (in-order and out-of-order composite PKs);
  - prefetch_primary_key? — false for composite, true for single
    PK, including a cold-cache regression that exercises the
    dictionary fallback (would have caught the
    composite_primary_key? cardinality bug);
  - RETURNING — both the "user supplies all PK columns" and the
    "all PK columns defaulted" path on a CPK table with column
    defaults (would have caught the multi-DEFAULT regex bug);
  - find / find with multiple keys (preserves request order) /
    wrapped-array find / class-level destroy / predicate-builder
    where-by-PK-columns;
  - mixed-type CPK (String + Integer) round-trip and find/destroy.
- identity_primary_key_spec.rb's empty_insert_statement_value
  composite assertion is updated to expect the new SQL fragment
  instead of NotImplementedError.

Closes rsim#2648, supersedes rsim#2530.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@yahonda yahonda force-pushed the composite-pk-2648 branch from 2a7c64a to 80832a1 Compare May 5, 2026 12:06
@yahonda yahonda merged commit 09c23fd into rsim:master May 5, 2026
12 checks passed
@yahonda yahonda deleted the composite-pk-2648 branch May 5, 2026 12:18
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request May 5, 2026
`has_primary_key?` was an oracle-enhanced override of an abstract
adapter method that Rails itself removed in commit d1521719c5
("Removed support for accessing attributes on a
has_and_belongs_to_many join table", 2011-01-16) when the HABTM
attribute-access path that needed it was retired. oracle-enhanced
kept the override, but it has been dead surface for 14+ years:

- No in-tree caller. The only previous user, the original
  `prefetch_primary_key?` cache-fill path, was refactored into
  `prefetch_primary_key_from_dictionary` and now goes through
  `primary_keys(table_name)` directly.
- Not part of the Rails abstract adapter API anymore. Rails callers
  use `primary_key(table_name)` / `primary_keys(table_name)` instead.
- Marked `# :nodoc:` so no API contract.
- The single spec assertion added in rsim#2693
  (`expect(@conn.has_primary_key?("uber_barcodes")).to be true`)
  duplicated the immediately preceding `primary_keys` assertion;
  removed alongside the method.

The body had also already lost its `(owner, desc_table_name)`
arguments (carried over from `pk_and_sequence_for`'s historical
signature, never functional because `pk_and_sequence_for` itself
unconditionally re-resolves them). Rather than keep a one-line
wrapper around `primary_keys(table_name).any?`, drop the method
entirely.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add composite primary key support (track Rails 7.1+ feature)

1 participant