Skip to content

Composite primary key support for write_lobs (CLOB/BLOB on CPK models) #2694

@yahonda

Description

@yahonda

Background

PR #2693 added composite primary key (CPK) support across the adapter, but OracleEnhanced::DatabaseStatements#write_lobs (lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb) still encodes a single-column primary-key assumption. A TODO comment was added there in the same PR; this issue tracks the actual fix.

Current code

def write_lobs(table_name, klass, attributes, columns) # :nodoc:
  id = quote(attributes[klass.primary_key])
  columns.each do |col|
    ...
    SELECT #{quote_column_name(col.name)} FROM #{quote_table_name(table_name)}
    WHERE #{quote_column_name(klass.primary_key)} = #{id} FOR UPDATE
    ...
  end
end

For a CPK model:

  • klass.primary_key is an Array (e.g. ["author_id", "id"]).
  • attributes[Array] returns nil.
  • quote_column_name(Array) produces a malformed SQL fragment such as "[\"author_id\", \"id\"]".

Both the id lookup and the WHERE clause are wrong, so any CPK model that has a CLOB / BLOB column will fail in:

  • insert_fixture (which calls write_lobs after super)
  • The post-save LOB rewrite path used when prepared_statements: false (or for any LOB column that wasn't sent via the regular bind path)

Repro sketch

ActiveRecord::Schema.define do
  create_table :cpk_docs, primary_key: [:author_id, :id], force: true do |t|
    t.integer :author_id
    t.integer :id
    t.text    :body  # CLOB
  end
end

class CpkDoc < ActiveRecord::Base
  self.primary_key = [:author_id, :id]
end

CpkDoc.create!(id: [1, 1], body: "x" * 5000)
# => fails when write_lobs runs, because attributes[[:author_id, :id]] is nil
#    and the WHERE clause becomes WHERE "["author_id", "id"]" = ...

Fix sketch

Build the lookup key and the WHERE clause from the primary-key components:

def write_lobs(table_name, klass, attributes, columns) # :nodoc:
  pk_cols = Array(klass.primary_key)
  pk_pairs = pk_cols.map { |col| [col, attributes[col]] }
  where_clause = pk_pairs.map { |col, val| "#{quote_column_name(col)} = #{quote(val)}" }.join(" AND ")

  columns.each do |col|
    ...
    SELECT #{quote_column_name(col.name)} FROM #{quote_table_name(table_name)}
    WHERE #{where_clause} FOR UPDATE
    ...
  end
end

Specs should cover:

  1. CPK + CLOB column, value above the varchar2 inline limit so the LOB rewrite path is exercised.
  2. CPK + BLOB column.
  3. The prepared_statements: false connection variant for the same scenarios.
  4. insert_fixture against a CPK + LOB table (covers the insert_fixture call site).

Out of scope

  • Composite-PK behaviour for non-LOB columns is already in Add composite primary key support #2693.
  • Other call sites that still encode the single-column-PK assumption should be tracked in their own issues if discovered.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions