googleapis / googleapis/ruby-spanner-activerecord

upsert_all / insert_all! (mutation path) corrupt non-integer composite primary key columns by casting them with BigInteger

Abierto
#407 0 comentarios 0 reacciones 1 asignado Reclamado por @olavloite Ver en GitHub
api: spanner priority: p2 type: bug
Lenguaje dominante
Ruby
Estrellas
105
Forks
37
Merge medio
10 h 2 min
PR fusionados (30 d)
5

Descripción

#### Environment details

- Programming language: Ruby
- OS: Linux (also reproduced on Spanner emulator and a real Spanner database)
- Language runtime version: Ruby 3.4.8
- Package version: `activerecord-spanner-adapter` 2.5.0 (also present on `main` — see `_set_composite_primary_key_value`)
- Rails: 8.0.4
- Spanner dialect: GoogleSQL

#### Steps to reproduce

1. Create a table with a **composite primary key that includes STRING columns** (minimal fictional schema below).
2. Outside a DML transaction (default mutation / `InsertOrUpdate` path), call `Model.upsert_all([row])` or `Model.insert_all!([row])` with a hash of plain Ruby values.
3. Read the row back and inspect the primary key columns.

```ruby
create_table :demo_items, id: false, force: true do |t|
t.integer :account_id, null: false
t.string :resource_uuid, null: false # UUID string
t.string :kind, null: false # e.g. "email"
t.string :value, null: false # e.g. "alice@example.com"
t.string :resource_type, null: false # non-PK column
t.datetime :created_at, null: false
end

execute <<~SQL
ALTER TABLE demo_items
ADD PRIMARY KEY (account_id, resource_uuid, kind, value)
SQL

class DemoItem < ActiveRecord::Base
self.table_name = "demo_items"
self.primary_key = %w[account_id resource_uuid kind value]
end

account_id = 4242
resource_uuid = "89ff73aa-c1cb-475e-bb27-89281c50c5b8"
created_at = Time.utc(2026, 1, 1, 12, 0, 0)

DemoItem.where(account_id: account_id).delete_all

row = {
"account_id" => account_id,
"resource_uuid" => resource_uuid,
"kind" => "email",
"value" => "alice@example.com",
"resource_type" => "house",
"created_at" => created_at,
}

# Mutation path (default when outside a DML transaction)
DemoItem.upsert_all([row])
# DemoItem.insert_all!([row]) # same corruption on the mutation path

puts DemoItem.where(account_id: account_id).map(&:attributes)
```

### Actual result

```ruby
[{
"account_id" => 4242,
"resource_uuid" => "89", # truncated via BigInteger.cast
"kind" => "0", # "email" => 0
"value" => "0", # "alice@example.com" => 0
"resource_type" => "house", # non-PK: OK
"created_at" => 2026-01-01 12:00:00 UTC, # non-PK: OK
}]
```

Lookup by the intended PK returns `nil`:

```ruby
DemoItem.find_by(
account_id: account_id,
resource_uuid: resource_uuid,
kind: "email",
value: "alice@example.com"
)
# => nil
```

### Expected result

Row persisted with the original STRING PK values (`resource_uuid`, `kind`, `value` unchanged).

## Root cause

In [`lib/activerecord_spanner_adapter/base.rb`](https://github.com/googleapis/ruby-spanner-activerecord/blob/main/lib/activerecord_spanner_adapter/base.rb), `_set_composite_primary_key_value` always defaults the type to `BigInteger` unless the caller already wrapped the value in `ActiveModel::Attribute`:

```ruby
def self._set_composite_primary_key_value primary_key, values, is_mutation
value = values[primary_key]
type = ActiveModel::Type::BigInteger.new # <-- always, for every composite PK column

if value.is_a? ActiveModel::Attribute
type = value.type
value = value.value
end
# ...
values[primary_key] =
ActiveModel::Attribute.from_database primary_key, value, type
end
```

Cast examples:

| Input | `BigInteger.cast` |
|---|---|
| `"89ff73aa-c1cb-..."` | `89` |
| `"email"` | `0` |
| `"alice@example.com"` | `0` |
| `4242` | `4242` (integer PK column survives) |

This path is used by `_buffer_record` for mutation writes (`insert` / `insert_or_update`), which is what `upsert_all` and `insert_all!` use by default when there is no open DML transaction (or when isolation is `:buffered_mutations`).

Inside a default ActiveRecord DML transaction, the same call goes through SQL DML and does **not** hit this cast — so the bug is mutation-path-specific.

`insert_all` (without bang) uses DML `INSERT OR IGNORE` and is not affected; under `:buffered_mutations` the adapter raises `NotImplementedError` for that method.

## Suggested fix

When rewriting composite PK values, use the **column's actual ActiveRecord type** (e.g. `type_for_attribute(primary_key)` / `columns_hash[primary_key].type`) instead of hard-coding `ActiveModel::Type::BigInteger`, at least when a non-nil client-provided value is present.

Integer / auto-generated PK columns would keep working; STRING (and other non-INT64) composite PK columns would stop being corrupted.

## Workaround (application-side)

Wrap each composite PK column with the real column type **before** `upsert_all` / `insert_all!` on the **mutation** path only:

```ruby
row = {
"account_id" => 4242,
"resource_uuid" => "89ff73aa-c1cb-475e-bb27-89281c50c5b8",
"kind" => "email",
"value" => "alice@example.com",
"resource_type" => "house",
"created_at" => Time.utc(2026, 1, 1, 12, 0, 0),
}

Array(DemoItem.primary_key).each do |column|
row[column] = ActiveModel::Attribute.from_database(
column,
row[column],
DemoItem.type_for_attribute(column)
)
end

DemoItem.upsert_all([row])
# => persists correct PK values on the mutation path
```

**Important:** do **not** apply this wrapping inside a DML transaction. In that path, `ActiveModel::Attribute` wrappers can be persisted as `NULL`. Only type PK columns when there is no open DML transaction (or when isolation is `:buffered_mutations`).

`stringify_keys` alone does **not** fix it; only pre-wrapping with the correct `ActiveModel::Attribute` type does.

## Notes

- Reproduced on Spanner emulator and on a real Spanner instance (so this is not an emulator-only quirk).
- Happy to contribute a failing test + patch if that would help.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.