From 3e418ceae088ffdbfe61f6f692f334c0ae216030 Mon Sep 17 00:00:00 2001 From: Derrick Austin Date: Thu, 11 Jun 2026 08:13:02 -0400 Subject: [PATCH] Bind attach relation keys as strings to use the attachment_id index The files table stores attachment_id as a string, but addConstraints() and addEagerConstraints() bound the parent key(s) using their native integer type. Comparing a string column against an integer forces the database to coerce the column value for every candidate row, which prevents the attachment_id index from being used - on MySQL/MariaDB an EXPLAIN shows the query falling back to the low-cardinality attachment_type index and examining every row of that morph type. Casting the keys to strings restores index usage for both lazy and eager attachment loads. getRelationExistenceQuery() already handles this for has() queries by casting the parent column to TEXT, so this brings the regular constraint paths in line with that approach. --- .../Relations/Concerns/AttachOneOrMany.php | 21 +++++- tests/Database/Relations/AttachOneTest.php | 75 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/Database/Relations/Concerns/AttachOneOrMany.php b/src/Database/Relations/Concerns/AttachOneOrMany.php index ff07ae3be..591c5e248 100644 --- a/src/Database/Relations/Concerns/AttachOneOrMany.php +++ b/src/Database/Relations/Concerns/AttachOneOrMany.php @@ -42,7 +42,12 @@ public function addConstraints() if (static::$constraints) { $this->query->where($this->morphType, $this->morphClass); - $this->query->where($this->foreignKey, '=', $this->getParentKey()); + // Bind the parent key as a string to match the string `attachment_id` column. A numeric + // binding forces the database to coerce the column for every row, which prevents the + // column's index from being used. + $parentKey = $this->getParentKey(); + + $this->query->where($this->foreignKey, '=', is_null($parentKey) ? $parentKey : (string) $parentKey); $this->query->where('field', $this->getFieldName()); @@ -116,7 +121,19 @@ public function getRelationExistenceQueryForSelfRelation(Builder $query, Builder */ public function addEagerConstraints(array $models) { - parent::addEagerConstraints($models); + // Bind the parent keys as strings to match the string `attachment_id` column, rather than + // deferring to the parent implementation, which compares integer keys against the string + // column and prevents the column's index from being used. Null keys are preserved as-is + // so they match nothing, instead of being cast to an empty string. + $this->query->whereIn( + $this->foreignKey, + array_map( + fn ($key) => is_null($key) ? $key : (string) $key, + $this->getKeys($models, $this->localKey) + ) + ); + + $this->query->where($this->morphType, $this->morphClass); $this->query->where('field', $this->fieldName); } diff --git a/tests/Database/Relations/AttachOneTest.php b/tests/Database/Relations/AttachOneTest.php index 2ed5e47d0..db4740977 100644 --- a/tests/Database/Relations/AttachOneTest.php +++ b/tests/Database/Relations/AttachOneTest.php @@ -2,6 +2,7 @@ namespace Winter\Storm\Tests\Database\Relations; +use Illuminate\Database\Eloquent\Relations\Relation; use Symfony\Component\HttpFoundation\File\UploadedFile; use Winter\Storm\Database\Attach\File; use Winter\Storm\Database\Model; @@ -95,6 +96,80 @@ public function testSetRelationValueLaravelRelation() $this->assertEquals('avatar.png', $user2->displayPicture->file_name); } + public function testConstraintsBindParentKeyAsString() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $bindings = $user->avatar()->getBaseQuery()->getBindings(); + + // The attachment_id column is a string; the key must be bound as a string so the + // database can use the column's index instead of coercing every row to a number. + $this->assertContains((string) $user->id, $bindings); + $this->assertNotContains($user->id, $bindings); + } + + public function testEagerConstraintsBindParentKeysAsStrings() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + $user2 = User::create(['name' => 'Joe', 'email' => 'joe@example.com']); + Model::reguard(); + + $relation = Relation::noConstraints(function () use ($user) { + return $user->avatar(); + }); + $relation->addEagerConstraints([$user, $user2]); + + $bindings = $relation->getBaseQuery()->getBindings(); + + $this->assertContains((string) $user->id, $bindings); + $this->assertContains((string) $user2->id, $bindings); + $this->assertNotContains($user->id, $bindings); + $this->assertNotContains($user2->id, $bindings); + } + + public function testEagerConstraintsPreserveNullParentKeys() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $unsavedUser = new User; + + $relation = Relation::noConstraints(function () use ($user) { + return $user->avatar(); + }); + $relation->addEagerConstraints([$user, $unsavedUser]); + + $bindings = $relation->getBaseQuery()->getBindings(); + + // A parent without a key must not be cast to an empty string, which could match + // orphaned rows where attachment_id = '' + $this->assertNotContains('', $bindings); + $this->assertContains((string) $user->id, $bindings); + } + + public function testEagerLoadMatchesAttachmentsToParents() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + $user2 = User::create(['name' => 'Joe', 'email' => 'joe@example.com']); + Model::reguard(); + + $user->avatar()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + $user2->avatar()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + + $results = User::with('avatar')->whereIn('id', [$user->id, $user2->id])->get(); + + foreach ($results as $result) { + $this->assertTrue($result->relationLoaded('avatar')); + $this->assertNotNull($result->avatar); + $this->assertEquals($result->id, $result->avatar->attachment_id); + } + } + public function testDeleteFlagDestroyRelationship() { Model::unguard();