Skip to content

fix(database): attach batch inserted relations to correct parent - #2320

Open
radoslav-grencik wants to merge 5 commits into
tempestphp:3.xfrom
radoslav-grencik:fix/batch-insert-relations-uuid
Open

radoslav-grencik wants to merge 5 commits into
tempestphp:3.xfrom
radoslav-grencik:fix/batch-insert-relations-uuid

Conversation

@radoslav-grencik

Copy link
Copy Markdown
Contributor

Summary

Batch inserts with relations silently attached every row's children to the
first parent: insert($a, $b) ran all relation callbacks with $a's
primary key, so $b's HasMany / HasOne / BelongsToMany children were written
with the wrong foreign key.

query(User::class)->insert(
    UuidUser::new(name: 'Frieren', posts: [new UuidPost(title: 'A')]),
    UuidUser::new(name: 'Fern', posts: [new UuidPost(title: 'B')]),
)->execute();

Before: both posts got Frieren's id.
After: each post gets its own parent's id.

Implementation

Relation callbacks are now grouped per row during data resolution
(relationCallbacks, sliced per resolved row alongside resolvedRows).
At execution time each row's callbacks receive that row's id:

  • UUID primary keys: the id is generated client-side before the INSERT
    runs, so it is read back from the resolved row data
    (resolveRowPrimaryKey()) and passed to the row's own callbacks.
    No dialect-specific behavior involved.
  • Auto-increment primary keys: intentionally keep the previous first-row
    behavior. Per-row ids are not reliably recoverable from a multi-row
    INSERT across dialects — MySQL returns the first auto-increment id,
    SQLite the last one, and PostgreSQL RETURNING has no guaranteed row
    order to map back to input rows. Supporting that correctly would require
    falling back to per-row inserts inside a transaction; out of scope until
    a concrete use case appears.

then() callbacks are unaffected (still receive the execute-time id).

…n keys

Iterable inserts (`insert()` with array data) never generated `#[Uuid]`
primary keys — only the object path (`create()`, `save()`) did — so UUID
models inserted this way ended up with a NULL key (error 1364 on MySQL).

Migrations also had no way to define UUID-typed foreign key columns:
`belongsTo()`/`foreignId()` always compile INTEGER, which cannot
reference a UUID primary key (fails on PostgreSQL).

- generate `#[Uuid]` values in the iterable insert path when the key
  is absent (explicitly provided ids are never overwritten)
- add `uuidColumn()`, `belongsToUuid()` and `foreignUuid()` to
  `CreateTableStatement` (CHAR(36) / UUID / TEXT per dialect)
@github-actions

Copy link
Copy Markdown

Benchmark Results

Comparison of fix/batch-insert-relations-uuid against 3.x (1509573a2148600c284435dde8761638d30aa2b9).

Open to see the benchmark results
Benchmark Set Mem. Peak Time Variability
ViewRenderBench(benchPlainHtml) - 22.182mb 0.00% 496.798μs -10.98% ±1.63% -36.45%
ViewRenderBench(benchExpressions) - 24.772mb 0.00% 518.315μs -12.60% ±1.97% -4.69%
ContainerBench(benchRegisterDefinition) - 6.786mb 0.00% 1.827μs +5.71% ±0.91% -0.59%

Generated by phpbench against commit ba61e50

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