Skip to content

Remove has_primary_key?#2695

Merged
yahonda merged 1 commit into
rsim:masterfrom
yahonda:has-primary-key-owner
May 5, 2026
Merged

Remove has_primary_key?#2695
yahonda merged 1 commit into
rsim:masterfrom
yahonda:has-primary-key-owner

Conversation

@yahonda

@yahonda yahonda commented May 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remove OracleEnhancedAdapter#has_primary_key? entirely. The method 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.

Why now / why outright removal

  • Zero in-tree callers. 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 in the Rails abstract API. Current Rails callers use primary_key(table_name) / primary_keys(table_name) instead.
  • # :nodoc: — no API contract is being broken.
  • 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 via resolve_data_source_name). The only thing left would have been a one-line wrapper around primary_keys(table_name).any? — not worth keeping.
  • The single spec assertion added in Add composite primary key support #2693 (expect(@conn.has_primary_key?("uber_barcodes")).to be true) duplicated the immediately preceding primary_keys assertion; removed alongside the method.

Diff

- def has_primary_key?(table_name, owner = nil, desc_table_name = nil) # :nodoc:
-   primary_keys(table_name).any?
- end
-
  def primary_keys(table_name) # :nodoc:

(Plus the deletion of one redundant spec line.)

Test plan

  • bundle exec rspec — full suite, 803 examples, 0 failures, 12 pre-existing pending
  • bundle exec rubocop — clean
  • No remaining references to has_primary_key? in lib/ or spec/

`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>
@yahonda yahonda force-pushed the has-primary-key-owner branch from fc9deb1 to 47b3a6b Compare May 5, 2026 12:34
@yahonda yahonda changed the title Drop unused owner / desc_table_name args from has_primary_key? Remove has_primary_key? May 5, 2026
@yahonda yahonda merged commit 93bd68f into rsim:master May 5, 2026
15 of 16 checks passed
@yahonda yahonda deleted the has-primary-key-owner branch May 5, 2026 12:56
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.

1 participant