Compare attachment foreign keys as strings so their index is used

[AIC-BV]

attachment_id is a string column, but AttachOneOrMany::addConstraints() binds the parent key as an integer, and eager loading inlines integer keys through Laravel's whereIntegerInRaw() (attachment_id in (1, 2, 3)). When MySQL compares a string column with a number, it converts every row's value, so it can't use attachment_id_index. It falls back to the attachment_type or field index and reads every attachment of that model class, or every file in that field. The existence queries in the same trait already cast the parent key with DbDongle::cast(..., 'TEXT'); this does the same for the plain and eager lookups.

Median times on MySQL 8.0.45. The files table uses Winter's system_files columns and indexes, filled with synthetic rows across 40 model classes, and storm's per-request query memo is disabled:

Rows in files	$user->avatar()->get() develop	this PR	with('avatar'), 25 parents, develop	this PR	lookup EXPLAIN, develop	lookup EXPLAIN, this PR
1,000	4.3 ms	3.6 ms	6.8 ms	6.5 ms	index_merge, 6 rows	ref, 1 row
10,000	4.8 ms	4.0 ms	6.9 ms	6.4 ms	index_merge, 27 rows	ref, 1 row
100,000	15 ms	7.1 ms	18 ms	12 ms	ref, 2,510 rows	ref, 1 row
250,000	25 ms	7.0 ms	25 ms	10 ms	index_merge, 1,791 rows	ref, 1 row
500,000	37 ms	7.4 ms	38 ms	11 ms	index_merge, 3,534 rows	ref, 1 row
1,000,000	400 ms	7.6 ms	360 ms	11 ms	ref, 46,226 rows	ref, 1 row

How bad it gets depends on how files are spread over classes and fields. Real tables are usually far more skewed than this synthetic one. On our production site's system_files (96k rows, a third of them attached to invoices), a single invoice PDF lookup takes 163 ms with an integer key, reading 32,963 field = 'pdf' index entries. With a string key it takes 0.05 ms and reads 1 row (EXPLAIN ANALYZE).

Change

• addConstraints() binds the parent key as a string. A null key stays null.
• addEagerConstraints() keeps Laravel's approach for integer keys: they are still inlined, with no placeholder per key, but as quoted string literals (in ('1', '2', '3')). Using bindings instead would hit the placeholder limits on large eager loads, such as 999 on older SQLite. Non-integer keys go through whereIn() as before.

Results are identical: attachment_id is always written as a string, and the eager-load dictionary matches '1' and 1 to the same key.

Tests

• testParentKeyIsBoundAsString and testEagerLoadInlinesParentKeysAsStrings both fail on develop.
• testEagerLoadWithoutParents checks that an empty key list still compiles to valid SQL.

This is independent of #251, but goes with it. #251 removes the OR that forces a full scan in withDeferred(), and with that gone, this integer key is what's left stopping MySQL from using attachment_id_index (the 1M-row step in that PR's chart). The benchmark, runner and raw results are on AIC-BV/storm@bench/attach-string-key.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8f95a9be-439a-42fb-842d-1a5814b94713

📥 Commits

Reviewing files that changed from the base of the PR and between e97198d and aa0027d.

📒 Files selected for processing (2)

• src/Database/Relations/Concerns/AttachOneOrMany.php
• tests/Database/Relations/AttachOneTest.php

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}