From 3ce950eaf6a4954b00c05647839c6f1afccf01e6 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:18:26 +0000 Subject: [PATCH 1/7] fix(database): make relationship collision recovery truthful Route create-or-first collision fallback reads through the write PDO so a configured replica cannot hide the winning row. Verify BelongsToMany pivot collisions against the exact parent, related model, pivot constraints, and morph discriminator before reporting attachment success. Rethrow the original attach violation when that membership cannot be proven. Correct return annotations for create, save, and create-or-retrieve paths that attach pivot rows without hydrating a pivot model. Add focused unit coverage, a SQLite read/write split regression, and shared four-engine collision coverage. --- .../src/Eloquent/Relations/BelongsToMany.php | 50 ++-- .../Relations/HasOneOrManyThrough.php | 3 +- ...EloquentBelongsToManyCreateOrFirstTest.php | 254 ++++++++++++++++-- ...loquentHasManyThroughCreateOrFirstTest.php | 6 +- ...elongsToManyCreateOrFirstCollisionTest.php | 134 +++++++++ .../EloquentCreateOrFirstReadWriteTest.php | 223 +++++++++++++++ 6 files changed, 632 insertions(+), 38 deletions(-) create mode 100644 tests/Integration/Database/EloquentBelongsToManyCreateOrFirstCollisionTest.php create mode 100644 tests/Integration/Database/Sqlite/EloquentCreateOrFirstReadWriteTest.php diff --git a/src/database/src/Eloquent/Relations/BelongsToMany.php b/src/database/src/Eloquent/Relations/BelongsToMany.php index ce3c70dee..bd86ee5a1 100644 --- a/src/database/src/Eloquent/Relations/BelongsToMany.php +++ b/src/database/src/Eloquent/Relations/BelongsToMany.php @@ -559,7 +559,7 @@ public function orderByPivotDesc(mixed $column): static * @return ( * $id is (\Hypervel\Contracts\Support\Arrayable|array) * ? \Hypervel\Database\Eloquent\Collection - * : TRelatedModel&object{pivot: TPivotModel} + * : TRelatedModel * ) */ public function findOrNew(mixed $id, array $columns = ['*']): EloquentCollection|Model @@ -574,7 +574,7 @@ public function findOrNew(mixed $id, array $columns = ['*']): EloquentCollection /** * Get the first related model record matching the attributes or instantiate it. * - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function firstOrNew(array $attributes = [], Closure|array $values = []): Model { @@ -588,7 +588,7 @@ public function firstOrNew(array $attributes = [], Closure|array $values = []): /** * Get the first record matching the attributes. If the record is not found, create it. * - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function firstOrCreate(array $attributes = [], Closure|array $values = [], array $joining = [], bool $touch = true): Model { @@ -598,8 +598,10 @@ public function firstOrCreate(array $attributes = [], Closure|array $values = [] } else { try { $this->getQuery()->withSavepointIfNeeded(fn () => $this->attach($instance, $joining, $touch)); - } catch (UniqueConstraintViolationException) { - // Nothing to do, the model was already attached... + } catch (UniqueConstraintViolationException $exception) { + if (! $this->hasAttachedPivot($instance)) { + throw $exception; + } } } } @@ -610,29 +612,43 @@ public function firstOrCreate(array $attributes = [], Closure|array $values = [] /** * Attempt to create the record. If a unique constraint violation occurs, attempt to find the matching record. * - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function createOrFirst(array $attributes = [], Closure|array $values = [], array $joining = [], bool $touch = true): Model { try { return $this->getQuery()->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, value($values)), $joining, $touch)); - } catch (UniqueConstraintViolationException $e) { + } catch (UniqueConstraintViolationException $exception) { // ... } + $instance = $this->related->where($attributes)->useWritePdo()->first() ?? throw $exception; + try { - return tap($this->related->where($attributes)->first() ?? throw $e, function ($instance) use ($joining, $touch) { - $this->getQuery()->withSavepointIfNeeded(fn () => $this->attach($instance, $joining, $touch)); - }); - } catch (UniqueConstraintViolationException $e) { - return (clone $this)->useWritePdo()->where($attributes)->first() ?? throw $e; + $this->getQuery()->withSavepointIfNeeded(fn () => $this->attach($instance, $joining, $touch)); + } catch (UniqueConstraintViolationException $attachException) { + if (! $this->hasAttachedPivot($instance)) { + throw $attachException; + } } + + return $instance; + } + + /** + * Determine if the related model is attached through the current pivot constraints. + */ + protected function hasAttachedPivot(Model $instance): bool + { + return $this->newPivotStatementForId($instance->getKey()) + ->useWritePdo() + ->exists(); } /** * Create or update a related record matching the attributes, and fill it with values. * - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function updateOrCreate(array $attributes, Closure|array $values = [], array $joining = [], bool $touch = true): Model { @@ -1197,7 +1213,7 @@ public function allRelatedIds(): BaseCollection * Save a new model and attach it to the parent model. * * @param TRelatedModel $model - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function save(Model $model, array $pivotAttributes = [], bool $touch = true): Model { @@ -1212,7 +1228,7 @@ public function save(Model $model, array $pivotAttributes = [], bool $touch = tr * Save a new model without raising any events and attach it to the parent model. * * @param TRelatedModel $model - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function saveQuietly(Model $model, array $pivotAttributes = [], bool $touch = true): Model { @@ -1258,7 +1274,7 @@ public function saveManyQuietly(iterable $models, array $pivotAttributes = []): /** * Create a new instance of the related model. * - * @return object{pivot: TPivotModel}&TRelatedModel + * @return TRelatedModel */ public function create(array $attributes = [], array $joining = [], bool $touch = true): Model { @@ -1279,7 +1295,7 @@ public function create(array $attributes = [], array $joining = [], bool $touch /** * Create an array of new instances of the related models. * - * @return array + * @return array */ public function createMany(iterable $records, array $joinings = []): array { diff --git a/src/database/src/Eloquent/Relations/HasOneOrManyThrough.php b/src/database/src/Eloquent/Relations/HasOneOrManyThrough.php index 972f5e61e..8519c93d2 100644 --- a/src/database/src/Eloquent/Relations/HasOneOrManyThrough.php +++ b/src/database/src/Eloquent/Relations/HasOneOrManyThrough.php @@ -223,7 +223,8 @@ public function createOrFirst(array $attributes = [], Closure|array $values = [] try { return $this->getQuery()->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, value($values)))); } catch (UniqueConstraintViolationException $exception) { - return $this->where($attributes)->first() ?? throw $exception; + // @phpstan-ignore return.type (generic type lost through where()->first() chain) + return $this->useWritePdo()->where($attributes)->first() ?? throw $exception; } } diff --git a/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php b/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php index 99684d061..8cc31a43b 100644 --- a/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php +++ b/tests/Database/DatabaseEloquentBelongsToManyCreateOrFirstTest.php @@ -11,6 +11,7 @@ use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\BelongsToMany; +use Hypervel\Database\Eloquent\Relations\MorphToMany; use Hypervel\Database\Query\Builder as BaseBuilder; use Hypervel\Database\UniqueConstraintViolationException; use Hypervel\Support\CarbonImmutable; @@ -91,7 +92,7 @@ public function testCreateOrFirstMethodAssociatesExistingRelated(): void $source->getConnection() ->expects('select') - ->with('select * from "related_table" where ("attr" = ?) limit 1', ['foo'], true, []) + ->with('select * from "related_table" where ("attr" = ?) limit 1', ['foo'], false, []) ->andReturn([[ 'id' => 456, 'attr' => 'foo', @@ -184,7 +185,7 @@ public function testCreateOrFirstMethodRetrievesExistingRelatedAssociatedJustNow $source->getConnection() ->expects('select') - ->with('select * from "related_table" where ("attr" = ?) limit 1', ['foo'], true, []) + ->with('select * from "related_table" where ("attr" = ?) limit 1', ['foo'], false, []) ->andReturn([[ 'id' => 456, 'attr' => 'foo', @@ -204,20 +205,12 @@ public function testCreateOrFirstMethodRetrievesExistingRelatedAssociatedJustNow $source->getConnection() ->expects('select') ->with( - 'select "related_table".*, "pivot_table"."source_id" as "pivot_source_id", "pivot_table"."related_id" as "pivot_related_id" from "related_table" inner join "pivot_table" on "related_table"."id" = "pivot_table"."related_id" where "pivot_table"."source_id" = ? and ("attr" = ?) limit 1', - [123, 'foo'], + 'select exists(select * from "pivot_table" where "pivot_table"."source_id" = ? and "pivot_table"."related_id" in (?)) as "exists"', + [123, 456], false, [], ) - ->andReturn([[ - 'id' => 456, - 'attr' => 'foo', - 'val' => 'bar', - 'created_at' => '2023-01-01 00:00:00', - 'updated_at' => '2023-01-01 00:00:00', - 'pivot_source_id' => 123, - 'pivot_related_id' => 456, - ]]); + ->andReturn([['exists' => 1]]); $result = $source->related()->createOrFirst(['attr' => 'foo'], ['val' => 'bar']); $this->assertFalse($result->wasRecentlyCreated); @@ -227,13 +220,57 @@ public function testCreateOrFirstMethodRetrievesExistingRelatedAssociatedJustNow 'val' => 'bar', 'created_at' => '2023-01-01T00:00:00.000000Z', 'updated_at' => '2023-01-01T00:00:00.000000Z', - 'pivot' => [ - 'source_id' => 123, - 'related_id' => 456, - ], ], $result->toArray()); } + public function testCreateOrFirstMethodRethrowsAttachViolationWhenExactPivotIsMissing(): void + { + $source = new SourceModel; + $source->id = 123; + $source->exists = true; + $this->mockConnectionForModels([$source, new RelatedModel], 'SQLite'); + $source->getConnection()->shouldReceive('transactionLevel')->andReturn(0); + $source->getConnection()->shouldReceive('getName')->andReturn('sqlite'); + + $relatedSql = 'insert into "related_table" ("attr", "val", "updated_at", "created_at") values (?, ?, ?, ?)'; + $relatedBindings = ['foo', 'bar', '2023-01-01 00:00:00', '2023-01-01 00:00:00']; + + $source->getConnection()->expects('insert')->with($relatedSql, $relatedBindings)->andThrow( + new UniqueConstraintViolationException('sqlite', $relatedSql, $relatedBindings, new Exception) + ); + $source->getConnection()->expects('select')->with( + 'select * from "related_table" where ("attr" = ?) limit 1', + ['foo'], + false, + [], + )->andReturn([[ + 'id' => 456, + 'attr' => 'foo', + 'val' => 'bar', + 'created_at' => '2023-01-01 00:00:00', + 'updated_at' => '2023-01-01 00:00:00', + ]]); + + $pivotSql = 'insert into "pivot_table" ("related_id", "source_id") values (?, ?)'; + $pivotBindings = [456, 123]; + $attachException = new UniqueConstraintViolationException('sqlite', $pivotSql, $pivotBindings, new Exception); + + $source->getConnection()->expects('insert')->with($pivotSql, $pivotBindings)->andThrow($attachException); + $source->getConnection()->expects('select')->with( + 'select exists(select * from "pivot_table" where "pivot_table"."source_id" = ? and "pivot_table"."related_id" in (?)) as "exists"', + [123, 456], + false, + [], + )->andReturn([['exists' => 0]]); + + try { + $source->related()->createOrFirst(['attr' => 'foo'], ['val' => 'bar']); + $this->fail('Expected the pivot attach violation to be rethrown.'); + } catch (UniqueConstraintViolationException $exception) { + $this->assertSame($attachException, $exception); + } + } + public function testFirstOrCreateMethodRetrievesExistingRelatedAndAssociatesIt(): void { $source = new SourceModel; @@ -292,6 +329,104 @@ public function testFirstOrCreateMethodRetrievesExistingRelatedAndAssociatesIt() ], $result->toArray()); } + public function testFirstOrCreateMethodReturnsExistingRelatedWhenExactPivotExistsAfterAttachCollision(): void + { + $source = new SourceModel; + $source->id = 123; + $source->exists = true; + $this->mockConnectionForModels([$source, new RelatedModel], 'SQLite'); + $source->getConnection()->shouldReceive('transactionLevel')->andReturn(0); + $source->getConnection()->shouldReceive('getName')->andReturn('sqlite'); + + $source->getConnection()->expects('select')->with( + 'select "related_table".*, "pivot_table"."source_id" as "pivot_source_id", "pivot_table"."related_id" as "pivot_related_id" from "related_table" inner join "pivot_table" on "related_table"."id" = "pivot_table"."related_id" where "pivot_table"."source_id" = ? and ("attr" = ?) limit 1', + [123, 'foo'], + true, + [], + )->andReturn([]); + + $source->getConnection()->expects('select')->with( + 'select * from "related_table" where ("attr" = ?) limit 1', + ['foo'], + true, + [], + )->andReturn([[ + 'id' => 456, + 'attr' => 'foo', + 'val' => 'bar', + 'created_at' => '2023-01-01 00:00:00', + 'updated_at' => '2023-01-01 00:00:00', + ]]); + + $sql = 'insert into "pivot_table" ("related_id", "source_id") values (?, ?)'; + $bindings = [456, 123]; + + $source->getConnection()->expects('insert')->with($sql, $bindings)->andThrow( + new UniqueConstraintViolationException('sqlite', $sql, $bindings, new Exception) + ); + + $source->getConnection()->expects('select')->with( + 'select exists(select * from "pivot_table" where "pivot_table"."source_id" = ? and "pivot_table"."related_id" in (?)) as "exists"', + [123, 456], + false, + [], + )->andReturn([['exists' => 1]]); + + $result = $source->related()->firstOrCreate(['attr' => 'foo'], ['val' => 'bar']); + + $this->assertSame(456, $result->id); + $this->assertFalse($result->wasRecentlyCreated); + } + + public function testFirstOrCreateMethodRethrowsAttachViolationWhenExactPivotIsMissing(): void + { + $source = new SourceModel; + $source->id = 123; + $source->exists = true; + $this->mockConnectionForModels([$source, new RelatedModel], 'SQLite'); + $source->getConnection()->shouldReceive('transactionLevel')->andReturn(0); + $source->getConnection()->shouldReceive('getName')->andReturn('sqlite'); + + $source->getConnection()->expects('select')->with( + 'select "related_table".*, "pivot_table"."source_id" as "pivot_source_id", "pivot_table"."related_id" as "pivot_related_id" from "related_table" inner join "pivot_table" on "related_table"."id" = "pivot_table"."related_id" where "pivot_table"."source_id" = ? and ("attr" = ?) limit 1', + [123, 'foo'], + true, + [], + )->andReturn([]); + + $source->getConnection()->expects('select')->with( + 'select * from "related_table" where ("attr" = ?) limit 1', + ['foo'], + true, + [], + )->andReturn([[ + 'id' => 456, + 'attr' => 'foo', + 'val' => 'bar', + 'created_at' => '2023-01-01 00:00:00', + 'updated_at' => '2023-01-01 00:00:00', + ]]); + + $sql = 'insert into "pivot_table" ("related_id", "source_id") values (?, ?)'; + $bindings = [456, 123]; + $attachException = new UniqueConstraintViolationException('sqlite', $sql, $bindings, new Exception); + + $source->getConnection()->expects('insert')->with($sql, $bindings)->andThrow($attachException); + $source->getConnection()->expects('select')->with( + 'select exists(select * from "pivot_table" where "pivot_table"."source_id" = ? and "pivot_table"."related_id" in (?)) as "exists"', + [123, 456], + false, + [], + )->andReturn([['exists' => 0]]); + + try { + $source->related()->firstOrCreate(['attr' => 'foo'], ['val' => 'bar']); + $this->fail('Expected the pivot attach violation to be rethrown.'); + } catch (UniqueConstraintViolationException $exception) { + $this->assertSame($attachException, $exception); + } + } + public function testFirstOrCreateMethodFallsBackToCreateOrFirst(): void { $source = new class extends SourceModel { @@ -564,6 +699,69 @@ protected function newBelongsToMany(Builder $query, Model $parent, $table, $fore $this->assertSame('baz', $result->val); } + public function testPivotMembershipCheckIncludesEveryConfiguredPivotConstraint(): void + { + $source = new SourceModel; + $source->id = 123; + $source->exists = true; + $related = new RelatedModel; + $related->id = 456; + $this->mockConnectionForModels([$source, $related], 'SQLite'); + + $relation = (new InspectableBelongsToMany( + $related->newQuery(), + $source, + 'pivot_table', + 'source_id', + 'related_id', + 'id', + 'id', + )) + ->wherePivot('status', 'active') + ->wherePivotIn('kind', ['primary', 'secondary']) + ->wherePivotNull('expired_at') + ->wherePivotBetween('score', [10, 20]); + + $source->getConnection()->expects('select')->with( + 'select exists(select * from "pivot_table" where ("status" = ? and "kind" in (?, ?) and "expired_at" is null and "score" between ? and ?) and "pivot_table"."source_id" = ? and "pivot_table"."related_id" in (?)) as "exists"', + ['active', 'primary', 'secondary', 10, 20, 123, 456], + false, + [], + )->andReturn([['exists' => 1]]); + + $this->assertTrue($relation->hasAttached($related)); + } + + public function testPivotMembershipCheckIncludesMorphDiscriminator(): void + { + $source = new SourceModel; + $source->id = 123; + $source->exists = true; + $related = new RelatedModel; + $related->id = 456; + $this->mockConnectionForModels([$source, $related], 'SQLite'); + + $relation = new InspectableMorphToMany( + $related->newQuery(), + $source, + 'source', + 'pivot_table', + 'source_id', + 'related_id', + 'id', + 'id', + ); + + $source->getConnection()->expects('select')->with( + 'select exists(select * from "pivot_table" where "pivot_table"."source_id" = ? and "source_type" = ? and "pivot_table"."related_id" in (?)) as "exists"', + [123, SourceModel::class, 456], + false, + [], + )->andReturn([['exists' => 1]]); + + $this->assertTrue($relation->hasAttached($related)); + } + protected function mockConnectionForModels(array $models, string $database, array $lastInsertIds = []): void { $grammarClass = 'Hypervel\Database\Query\Grammars\\' . $database . 'Grammar'; @@ -622,3 +820,25 @@ public function related(): BelongsToMany ); } } + +class InspectableBelongsToMany extends BelongsToMany +{ + /** + * Determine if the related model is attached through the current pivot constraints. + */ + public function hasAttached(Model $instance): bool + { + return $this->hasAttachedPivot($instance); + } +} + +class InspectableMorphToMany extends MorphToMany +{ + /** + * Determine if the related model is attached through the current morph constraints. + */ + public function hasAttached(Model $instance): bool + { + return $this->hasAttachedPivot($instance); + } +} diff --git a/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php b/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php index 9da98245f..ea9ad410d 100644 --- a/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php +++ b/tests/Database/DatabaseEloquentHasManyThroughCreateOrFirstTest.php @@ -88,7 +88,7 @@ public function testCreateOrFirstMethodRetrievesExistingRecord(): void ->with( 'select "child".*, "pivot"."parent_id" as "hypervel_through_key" from "child" inner join "pivot" on "pivot"."id" = "child"."pivot_id" where "pivot"."parent_id" = ? and ("attr" = ?) limit 1', [123, 'foo'], - true, + false, [], ) ->andReturn([[ @@ -221,7 +221,7 @@ public function testFirstOrCreateMethodRetrievesRecordCreatedJustNow(): void ->with( 'select "child".*, "pivot"."parent_id" as "hypervel_through_key" from "child" inner join "pivot" on "pivot"."id" = "child"."pivot_id" where "pivot"."parent_id" = ? and ("attr" = ? and "val" = ?) limit 1', [123, 'foo', 'bar'], - true, + false, [], ) ->andReturn([[ @@ -365,7 +365,7 @@ public function testUpdateOrCreateMethodUpdatesRecordCreatedJustNow(): void ->with( 'select "child".*, "pivot"."parent_id" as "hypervel_through_key" from "child" inner join "pivot" on "pivot"."id" = "child"."pivot_id" where "pivot"."parent_id" = ? and ("attr" = ? and "val" = ?) limit 1', [123, 'foo', 'bar'], - true, + false, [], ) ->andReturn([[ diff --git a/tests/Integration/Database/EloquentBelongsToManyCreateOrFirstCollisionTest.php b/tests/Integration/Database/EloquentBelongsToManyCreateOrFirstCollisionTest.php new file mode 100644 index 000000000..94b8f8969 --- /dev/null +++ b/tests/Integration/Database/EloquentBelongsToManyCreateOrFirstCollisionTest.php @@ -0,0 +1,134 @@ +increments('id'); + }); + + Schema::create('collision_related', function (Blueprint $table): void { + $table->increments('id'); + $table->string('name')->unique(); + }); + + Schema::create('collision_pivot', function (Blueprint $table): void { + $table->integer('source_id'); + $table->integer('related_id'); + $table->string('collision_key')->unique(); + $table->unique(['source_id', 'related_id']); + }); + } + + public function testFirstOrCreateRethrowsAnIndependentPivotUniqueViolation(): void + { + [$source, $related] = $this->seedIndependentPivotCollision(); + + try { + $source->related()->firstOrCreate( + ['name' => $related->name], + joining: ['collision_key' => 'occupied'], + touch: false, + ); + + $this->fail('Expected the independent pivot unique violation to be rethrown.'); + } catch (UniqueConstraintViolationException) { + $this->assertExactPivotIsMissing($source, $related); + } + } + + public function testCreateOrFirstRethrowsAnIndependentPivotUniqueViolation(): void + { + [$source, $related] = $this->seedIndependentPivotCollision(); + + try { + $source->related()->createOrFirst( + ['name' => $related->name], + joining: ['collision_key' => 'occupied'], + touch: false, + ); + + $this->fail('Expected the independent pivot unique violation to be rethrown.'); + } catch (UniqueConstraintViolationException) { + $this->assertExactPivotIsMissing($source, $related); + } + } + + /** + * Seed a pivot row whose independent unique key will collide with the attempted attachment. + * + * @return array{CollisionSource, CollisionRelated} + */ + protected function seedIndependentPivotCollision(): array + { + $source = CollisionSource::create(); + $related = CollisionRelated::create(['name' => 'target']); + $otherSource = CollisionSource::create(); + $otherRelated = CollisionRelated::create(['name' => 'other']); + + DB::table('collision_pivot')->insert([ + 'source_id' => $otherSource->id, + 'related_id' => $otherRelated->id, + 'collision_key' => 'occupied', + ]); + + return [$source, $related]; + } + + /** + * Assert that the intended relation membership was not created. + */ + protected function assertExactPivotIsMissing(CollisionSource $source, CollisionRelated $related): void + { + $this->assertFalse( + DB::table('collision_pivot') + ->where('source_id', $source->id) + ->where('related_id', $related->id) + ->exists() + ); + } +} + +class CollisionSource extends Model +{ + protected ?string $table = 'collision_sources'; + + protected array $guarded = []; + + public bool $timestamps = false; + + /** + * Get the related models. + */ + public function related(): BelongsToMany + { + return $this->belongsToMany( + CollisionRelated::class, + 'collision_pivot', + 'source_id', + 'related_id', + ); + } +} + +class CollisionRelated extends Model +{ + protected ?string $table = 'collision_related'; + + protected array $guarded = []; + + public bool $timestamps = false; +} diff --git a/tests/Integration/Database/Sqlite/EloquentCreateOrFirstReadWriteTest.php b/tests/Integration/Database/Sqlite/EloquentCreateOrFirstReadWriteTest.php new file mode 100644 index 000000000..0e706aefa --- /dev/null +++ b/tests/Integration/Database/Sqlite/EloquentCreateOrFirstReadWriteTest.php @@ -0,0 +1,223 @@ +deleteDirectory(static::$databaseDirectory); + $filesystem->ensureDirectoryExists(static::$databaseDirectory); + + static::$readPath = static::$databaseDirectory . '/read.sqlite'; + static::$writePath = static::$databaseDirectory . '/write.sqlite'; + touch(static::$readPath); + touch(static::$writePath); + } + + public static function tearDownAfterClass(): void + { + (new Filesystem)->deleteDirectory(static::$databaseDirectory); + + parent::tearDownAfterClass(); + } + + protected function defineEnvironment(ApplicationContract $app): void + { + parent::defineEnvironment($app); + + $config = $app->make('config'); + + $config->set('database.connections.collision_read', [ + 'driver' => 'sqlite', + 'database' => static::$readPath, + 'prefix' => '', + ]); + $config->set('database.connections.collision_write', [ + 'driver' => 'sqlite', + 'database' => static::$writePath, + 'prefix' => '', + ]); + $config->set('database.connections.collision_split', [ + 'driver' => 'sqlite', + 'read' => ['database' => static::$readPath], + 'write' => ['database' => static::$writePath], + 'sticky' => false, + 'prefix' => '', + ]); + } + + protected function afterRefreshingDatabase(): void + { + $this->refreshSchema('collision_read'); + $this->refreshSchema('collision_write'); + } + + public function testHasManyThroughCollisionFallbackReadsTheWriter(): void + { + DB::connection('collision_write')->table('read_write_through')->insert([ + 'id' => 10, + 'source_id' => 1, + ]); + DB::connection('collision_write')->table('read_write_children')->insert([ + 'id' => 100, + 'through_id' => 10, + 'name' => 'winner', + ]); + + $source = new ReadWriteSource; + $source->id = 1; + $source->exists = true; + + $result = $source->children()->createOrFirst(['name' => 'winner']); + + $this->assertSame(100, $result->id); + $this->assertFalse($result->wasRecentlyCreated); + } + + public function testBelongsToManyCollisionFallbackReadsTheWriter(): void + { + DB::connection('collision_write')->table('read_write_related')->insert([ + 'id' => 200, + 'name' => 'winner', + ]); + DB::connection('collision_write')->table('read_write_pivot')->insert([ + 'source_id' => 1, + 'related_id' => 200, + ]); + + $source = new ReadWriteSource; + $source->id = 1; + $source->exists = true; + + $result = $source->related()->createOrFirst(['name' => 'winner'], touch: false); + + $this->assertSame(200, $result->id); + $this->assertFalse($result->wasRecentlyCreated); + } + + /** + * Recreate the read/write regression schema on one physical database. + */ + protected function refreshSchema(string $connection): void + { + $schema = Schema::connection($connection); + + $schema->dropIfExists('read_write_pivot'); + $schema->dropIfExists('read_write_related'); + $schema->dropIfExists('read_write_children'); + $schema->dropIfExists('read_write_through'); + + $schema->create('read_write_through', function (Blueprint $table): void { + $table->integer('id')->primary(); + $table->integer('source_id'); + }); + + $schema->create('read_write_children', function (Blueprint $table): void { + $table->integer('id')->primary(); + $table->integer('through_id')->nullable(); + $table->string('name')->unique(); + }); + + $schema->create('read_write_related', function (Blueprint $table): void { + $table->integer('id')->primary(); + $table->string('name')->unique(); + }); + + $schema->create('read_write_pivot', function (Blueprint $table): void { + $table->integer('source_id'); + $table->integer('related_id'); + $table->unique(['source_id', 'related_id']); + }); + } +} + +class ReadWriteSource extends Model +{ + protected UnitEnum|string|null $connection = 'collision_split'; + + protected ?string $table = 'read_write_sources'; + + public bool $timestamps = false; + + /** + * Get the children through the intermediate table. + */ + public function children(): HasManyThrough + { + return $this->hasManyThrough( + ReadWriteChild::class, + ReadWriteThrough::class, + 'source_id', + 'through_id', + ); + } + + /** + * Get the related models. + */ + public function related(): BelongsToMany + { + return $this->belongsToMany( + ReadWriteRelated::class, + 'read_write_pivot', + 'source_id', + 'related_id', + ); + } +} + +class ReadWriteThrough extends Model +{ + protected UnitEnum|string|null $connection = 'collision_split'; + + protected ?string $table = 'read_write_through'; + + public bool $timestamps = false; +} + +class ReadWriteChild extends Model +{ + protected UnitEnum|string|null $connection = 'collision_split'; + + protected ?string $table = 'read_write_children'; + + protected array $guarded = []; + + public bool $timestamps = false; +} + +class ReadWriteRelated extends Model +{ + protected UnitEnum|string|null $connection = 'collision_split'; + + protected ?string $table = 'read_write_related'; + + protected array $guarded = []; + + public bool $timestamps = false; +} From 4a92031b43306437ba514f3c14decd42a317476e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:18:40 +0000 Subject: [PATCH 2/7] fix(database): make query timeouts statement-owned Split raw select assembly from complete-statement decoration so MySQL and MariaDB apply query timeouts exactly once at the executed statement root. Cover unions, exists queries, grouped pagination, locking reads, and retained fragments while preserving each child builder's grammar and table prefix. Normalize Relation and Eloquent subqueries to one Query Builder snapshot before timeout checks, SQL compilation, bindings, or cross-database qualification. Reject timed embedded queries and timed EXPLAIN statements with clear diagnostics, and keep opaque or tableless sources safe during qualification. Align Query Builder's supported queryable input types, remove the obsolete SQLite group-limit fallback, preserve raw PostgreSQL and SQLite DML subselects, and add extensive grammar, builder, and real MySQL/MariaDB enforcement coverage. --- src/database/src/Concerns/ExplainsQueries.php | 11 +- src/database/src/Query/Builder.php | 181 ++++-- src/database/src/Query/Grammars/Grammar.php | 48 +- .../src/Query/Grammars/MariaDbGrammar.php | 12 + .../src/Query/Grammars/MySqlGrammar.php | 11 +- .../src/Query/Grammars/PostgresGrammar.php | 4 +- .../src/Query/Grammars/SQLiteGrammar.php | 20 +- .../DatabaseMariaDbQueryGrammarTest.php | 82 ++- .../DatabaseMySqlQueryGrammarTest.php | 70 ++- tests/Database/DatabaseQueryBuilderTest.php | 588 ++++++++++++++++++ .../Database/MariaDb/QueryTimeoutTest.php | 22 + .../Database/MySql/QueryTimeoutTest.php | 22 + .../Database/QueryTimeoutTestCase.php | 111 ++++ 13 files changed, 1084 insertions(+), 98 deletions(-) create mode 100644 tests/Integration/Database/MariaDb/QueryTimeoutTest.php create mode 100644 tests/Integration/Database/MySql/QueryTimeoutTest.php create mode 100644 tests/Integration/Database/QueryTimeoutTestCase.php diff --git a/src/database/src/Concerns/ExplainsQueries.php b/src/database/src/Concerns/ExplainsQueries.php index 839c886fd..560193083 100644 --- a/src/database/src/Concerns/ExplainsQueries.php +++ b/src/database/src/Concerns/ExplainsQueries.php @@ -5,14 +5,23 @@ namespace Hypervel\Database\Concerns; use Hypervel\Support\Collection; +use InvalidArgumentException; trait ExplainsQueries { /** - * Explains the query. + * Explain the query. + * + * @throws InvalidArgumentException */ public function explain(): Collection { + if ($this->timeout !== null) { + throw new InvalidArgumentException( + 'A query timeout cannot be applied to an EXPLAIN statement. Clear the timeout before calling explain().' + ); + } + $sql = $this->toSql(); $bindings = $this->getBindings(); diff --git a/src/database/src/Query/Builder.php b/src/database/src/Query/Builder.php index 4c8fc9157..894bc541b 100644 --- a/src/database/src/Query/Builder.php +++ b/src/database/src/Query/Builder.php @@ -278,11 +278,11 @@ public function select(mixed $columns = ['*']): static /** * Add a subselect expression to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query * * @throws InvalidArgumentException */ - public function selectSub(Closure|self|EloquentBuilder|string $query, string $as): static + public function selectSub(Closure|self|EloquentBuilder|Relation|string $query, string $as): static { [$query, $bindings] = $this->createSub($query); @@ -319,11 +319,11 @@ public function selectRaw(string $expression, array $bindings = []): static /** * Makes "from" fetch from a subquery. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query * * @throws InvalidArgumentException */ - public function fromSub(Closure|self|EloquentBuilder|string $query, string $as): static + public function fromSub(Closure|self|EloquentBuilder|Relation|string $query, string $as): static { [$query, $bindings] = $this->createSub($query); @@ -347,9 +347,9 @@ public function fromRaw(Expression|string $expression, mixed $bindings = []): st /** * Creates a subquery and parse it. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query */ - protected function createSub(Closure|self|EloquentBuilder|string $query): array + protected function createSub(Closure|self|EloquentBuilder|Relation|string $query): array { // If the given query is a Closure, we will execute it while passing in a new // query instance to the Closure. This will give the developer a chance to @@ -370,7 +370,17 @@ protected function createSub(Closure|self|EloquentBuilder|string $query): array */ protected function parseSub(mixed $query): array { - if ($query instanceof self || $query instanceof EloquentBuilder || $query instanceof Relation) { + if ($query instanceof Relation) { + $query = $query->getQuery(); + } + + if ($query instanceof EloquentBuilder) { + $query = $query->toBase(); + } + + if ($query instanceof self) { + $this->assertNoTimeoutOnEmbeddedQuery($query); + $query = $this->prependDatabaseNameIfCrossDatabaseQuery($query); return [$query->toSql(), $query->getBindings()]; @@ -386,13 +396,15 @@ protected function parseSub(mixed $query): array /** * Prepend the database name if the given query is on another database. */ - protected function prependDatabaseNameIfCrossDatabaseQuery(self|EloquentBuilder|Relation $query): self|EloquentBuilder|Relation + protected function prependDatabaseNameIfCrossDatabaseQuery(self $query): self { if ($query->getConnection()->getDatabaseName() !== $this->getConnection()->getDatabaseName()) { $databaseName = $query->getConnection()->getDatabaseName(); - if (! str_starts_with($query->from, $databaseName) && ! str_contains($query->from, '.')) { + if (is_string($query->from) + && ! str_starts_with($query->from, $databaseName) + && ! str_contains($query->from, '.')) { $query->from($databaseName . '.' . $query->from); } } @@ -481,9 +493,9 @@ public function distinct(): static /** * Set the table which the query is targeting. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $table + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $table */ - public function from(Closure|self|EloquentBuilder|ExpressionContract|string $table, ?string $as = null): static + public function from(Closure|self|EloquentBuilder|Relation|ExpressionContract|string $table, ?string $as = null): static { if ($this->isQueryable($table)) { return $this->fromSub($table, $as); @@ -567,11 +579,11 @@ public function joinWhere(ExpressionContract|string $table, Closure|ExpressionCo /** * Add a "subquery join" clause to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query * * @throws InvalidArgumentException */ - public function joinSub(Closure|self|EloquentBuilder|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, mixed $second = null, string $type = 'inner', bool $where = false): static + public function joinSub(Closure|self|EloquentBuilder|Relation|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, mixed $second = null, string $type = 'inner', bool $where = false): static { [$query, $bindings] = $this->createSub($query); @@ -585,9 +597,9 @@ public function joinSub(Closure|self|EloquentBuilder|string $query, string $as, /** * Add a "lateral join" clause to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query */ - public function joinLateral(Closure|self|EloquentBuilder|string $query, string $as, string $type = 'inner'): static + public function joinLateral(Closure|self|EloquentBuilder|Relation|string $query, string $as, string $type = 'inner'): static { [$query, $bindings] = $this->createSub($query); @@ -603,9 +615,9 @@ public function joinLateral(Closure|self|EloquentBuilder|string $query, string $ /** * Add a lateral left join to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query */ - public function leftJoinLateral(Closure|self|EloquentBuilder|string $query, string $as): static + public function leftJoinLateral(Closure|self|EloquentBuilder|Relation|string $query, string $as): static { return $this->joinLateral($query, $as, 'left'); } @@ -629,9 +641,9 @@ public function leftJoinWhere(ExpressionContract|string $table, Closure|Expressi /** * Add a subquery left join to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query */ - public function leftJoinSub(Closure|self|EloquentBuilder|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static + public function leftJoinSub(Closure|self|EloquentBuilder|Relation|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static { return $this->joinSub($query, $as, $first, $operator, $second, 'left'); } @@ -655,9 +667,9 @@ public function rightJoinWhere(ExpressionContract|string $table, Closure|Express /** * Add a subquery right join to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|string $query + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|string $query */ - public function rightJoinSub(Closure|self|EloquentBuilder|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static + public function rightJoinSub(Closure|self|EloquentBuilder|Relation|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static { return $this->joinSub($query, $as, $first, $operator, $second, 'right'); } @@ -679,7 +691,7 @@ public function crossJoin(ExpressionContract|string $table, Closure|ExpressionCo /** * Add a subquery cross join to the query. */ - public function crossJoinSub(Closure|self|EloquentBuilder|string $query, string $as): static + public function crossJoinSub(Closure|self|EloquentBuilder|Relation|string $query, string $as): static { [$query, $bindings] = $this->createSub($query); @@ -711,9 +723,9 @@ public function straightJoinWhere(ExpressionContract|string $table, Closure|Expr /** * Add a subquery straight join to the query. * - * @param Closure|self|EloquentBuilder<*>|string $query + * @param Closure|self|EloquentBuilder<*>|Relation<*, *, *>|string $query */ - public function straightJoinSub(Closure|self|EloquentBuilder|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static + public function straightJoinSub(Closure|self|EloquentBuilder|Relation|string $query, string $as, Closure|ExpressionContract|string $first, ?string $operator = null, ExpressionContract|string|null $second = null): static { return $this->joinSub($query, $as, $first, $operator, $second, 'straight_join'); } @@ -919,7 +931,7 @@ protected function isBitwiseOperator(string $operator): bool /** * Add an "or where" clause to the query. */ - public function orWhere(Closure|string|array|ExpressionContract $column, mixed $operator = null, mixed $value = null): static + public function orWhere(Closure|self|EloquentBuilder|Relation|ExpressionContract|array|string $column, mixed $operator = null, mixed $value = null): static { [$value, $operator] = $this->prepareValueAndOperator( $value, @@ -933,7 +945,7 @@ public function orWhere(Closure|string|array|ExpressionContract $column, mixed $ /** * Add a basic "where not" clause to the query. */ - public function whereNot(Closure|string|array|ExpressionContract $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static + public function whereNot(Closure|self|EloquentBuilder|Relation|ExpressionContract|array|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { if (is_array($column)) { return $this->whereNested(function ($query) use ($column, $operator, $value, $boolean) { @@ -947,7 +959,7 @@ public function whereNot(Closure|string|array|ExpressionContract $column, mixed /** * Add an "or where not" clause to the query. */ - public function orWhereNot(Closure|string|array|ExpressionContract $column, mixed $operator = null, mixed $value = null): static + public function orWhereNot(Closure|self|EloquentBuilder|Relation|ExpressionContract|array|string $column, mixed $operator = null, mixed $value = null): static { return $this->whereNot($column, $operator, $value, 'or'); } @@ -1282,9 +1294,9 @@ public function whereNotNull(string|array|ExpressionContract $columns, string $b /** * Add a "where between" statement to the query. * - * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $column + * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $column */ - public function whereBetween(self|EloquentBuilder|ExpressionContract|string $column, iterable $values, string $boolean = 'and', bool $not = false): static + public function whereBetween(self|EloquentBuilder|Relation|ExpressionContract|string $column, iterable $values, string $boolean = 'and', bool $not = false): static { $type = 'between'; @@ -1309,9 +1321,9 @@ public function whereBetween(self|EloquentBuilder|ExpressionContract|string $col /** * Add a "where between" statement using columns to the query. * - * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $column + * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $column */ - public function whereBetweenColumns(self|EloquentBuilder|ExpressionContract|string $column, array $values, string $boolean = 'and', bool $not = false): static + public function whereBetweenColumns(self|EloquentBuilder|Relation|ExpressionContract|string $column, array $values, string $boolean = 'and', bool $not = false): static { $type = 'betweenColumns'; @@ -1330,9 +1342,9 @@ public function whereBetweenColumns(self|EloquentBuilder|ExpressionContract|stri /** * Add an "or where between" statement to the query. * - * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $column + * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $column */ - public function orWhereBetween(self|EloquentBuilder|ExpressionContract|string $column, iterable $values): static + public function orWhereBetween(self|EloquentBuilder|Relation|ExpressionContract|string $column, iterable $values): static { return $this->whereBetween($column, $values, 'or'); } @@ -1340,7 +1352,7 @@ public function orWhereBetween(self|EloquentBuilder|ExpressionContract|string $c /** * Add an "or where between" statement using columns to the query. */ - public function orWhereBetweenColumns(ExpressionContract|string $column, array $values): static + public function orWhereBetweenColumns(self|EloquentBuilder|Relation|ExpressionContract|string $column, array $values): static { return $this->whereBetweenColumns($column, $values, 'or'); } @@ -1348,9 +1360,9 @@ public function orWhereBetweenColumns(ExpressionContract|string $column, array $ /** * Add a "where not between" statement to the query. * - * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $column + * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $column */ - public function whereNotBetween(self|EloquentBuilder|ExpressionContract|string $column, iterable $values, string $boolean = 'and'): static + public function whereNotBetween(self|EloquentBuilder|Relation|ExpressionContract|string $column, iterable $values, string $boolean = 'and'): static { return $this->whereBetween($column, $values, $boolean, true); } @@ -1358,7 +1370,7 @@ public function whereNotBetween(self|EloquentBuilder|ExpressionContract|string $ /** * Add a "where not between" statement using columns to the query. */ - public function whereNotBetweenColumns(ExpressionContract|string $column, array $values, string $boolean = 'and'): static + public function whereNotBetweenColumns(self|EloquentBuilder|Relation|ExpressionContract|string $column, array $values, string $boolean = 'and'): static { return $this->whereBetweenColumns($column, $values, $boolean, true); } @@ -1366,9 +1378,9 @@ public function whereNotBetweenColumns(ExpressionContract|string $column, array /** * Add an "or where not between" statement to the query. * - * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Contracts\Database\Query\Expression|string $column + * @param \Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|string $column */ - public function orWhereNotBetween(self|EloquentBuilder|ExpressionContract|string $column, iterable $values): static + public function orWhereNotBetween(self|EloquentBuilder|Relation|ExpressionContract|string $column, iterable $values): static { return $this->whereNotBetween($column, $values, 'or'); } @@ -1376,7 +1388,7 @@ public function orWhereNotBetween(self|EloquentBuilder|ExpressionContract|string /** * Add an "or where not between" statement using columns to the query. */ - public function orWhereNotBetweenColumns(ExpressionContract|string $column, array $values): static + public function orWhereNotBetweenColumns(self|EloquentBuilder|Relation|ExpressionContract|string $column, array $values): static { return $this->whereNotBetweenColumns($column, $values, 'or'); } @@ -1705,9 +1717,11 @@ public function addNestedWhereQuery(self $query, string $boolean = 'and'): stati /** * Add a full sub-select to the query. * - * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*> $callback + * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *> $callback + * + * @throws InvalidArgumentException */ - protected function whereSub(ExpressionContract|string $column, string $operator, Closure|self|EloquentBuilder $callback, string $boolean): static + protected function whereSub(ExpressionContract|string $column, string $operator, Closure|self|EloquentBuilder|Relation $callback, string $boolean): static { $type = 'Sub'; @@ -1717,9 +1731,11 @@ protected function whereSub(ExpressionContract|string $column, string $operator, // in the array of where clauses for the "main" parent query instance. $callback($query = $this->forSubQuery()); } else { - $query = $callback instanceof EloquentBuilder ? $callback->toBase() : $callback; + $query = $callback instanceof self ? $callback : $callback->toBase(); } + $this->assertNoTimeoutOnEmbeddedQuery($query); + $this->wheres[] = compact( 'type', 'column', @@ -1737,6 +1753,8 @@ protected function whereSub(ExpressionContract|string $column, string $operator, * Add an "exists" clause to the query. * * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*> $callback + * + * @throws InvalidArgumentException */ public function whereExists(Closure|self|EloquentBuilder $callback, string $boolean = 'and', bool $not = false): static { @@ -1758,6 +1776,8 @@ public function whereExists(Closure|self|EloquentBuilder $callback, string $bool * Add an "or where exists" clause to the query. * * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*> $callback + * + * @throws InvalidArgumentException */ public function orWhereExists(Closure|self|EloquentBuilder $callback, bool $not = false): static { @@ -1768,6 +1788,8 @@ public function orWhereExists(Closure|self|EloquentBuilder $callback, bool $not * Add a "where not exists" clause to the query. * * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*> $callback + * + * @throws InvalidArgumentException */ public function whereNotExists(Closure|self|EloquentBuilder $callback, string $boolean = 'and'): static { @@ -1778,6 +1800,8 @@ public function whereNotExists(Closure|self|EloquentBuilder $callback, string $b * Add an "or where not exists" clause to the query. * * @param \Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*> $callback + * + * @throws InvalidArgumentException */ public function orWhereNotExists(Closure|self|EloquentBuilder $callback): static { @@ -1786,9 +1810,13 @@ public function orWhereNotExists(Closure|self|EloquentBuilder $callback): static /** * Add an "exists" clause to the query. + * + * @throws InvalidArgumentException */ public function addWhereExistsQuery(self $query, string $boolean = 'and', bool $not = false): static { + $this->assertNoTimeoutOnEmbeddedQuery($query); + $type = $not ? 'NotExists' : 'Exists'; $this->wheres[] = compact('type', 'query', 'boolean'); @@ -2067,7 +2095,7 @@ public function orWhereFullText(string|array $columns, string $value, array $opt /** * Add a "where" clause to the query for multiple columns with "and" conditions between them. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function whereAll(array $columns, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { @@ -2089,7 +2117,7 @@ public function whereAll(array $columns, mixed $operator = null, mixed $value = /** * Add an "or where" clause to the query for multiple columns with "and" conditions between them. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function orWhereAll(array $columns, mixed $operator = null, mixed $value = null): static { @@ -2099,7 +2127,7 @@ public function orWhereAll(array $columns, mixed $operator = null, mixed $value /** * Add a "where" clause to the query for multiple columns with "or" conditions between them. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function whereAny(array $columns, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { @@ -2121,7 +2149,7 @@ public function whereAny(array $columns, mixed $operator = null, mixed $value = /** * Add an "or where" clause to the query for multiple columns with "or" conditions between them. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function orWhereAny(array $columns, mixed $operator = null, mixed $value = null): static { @@ -2131,7 +2159,7 @@ public function orWhereAny(array $columns, mixed $operator = null, mixed $value /** * Add a "where not" clause to the query for multiple columns where none of the conditions should be true. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function whereNone(array $columns, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { @@ -2141,7 +2169,7 @@ public function whereNone(array $columns, mixed $operator = null, mixed $value = /** * Add an "or where not" clause to the query for multiple columns where none of the conditions should be true. * - * @param array $columns + * @param array|Relation<*, *, *>|ExpressionContract|string> $columns */ public function orWhereNone(array $columns, mixed $operator = null, mixed $value = null): static { @@ -2396,12 +2424,12 @@ public function orHavingRaw(string $sql, array $bindings = []): static /** * Add an "order by" clause to the query. * - * @param Closure|self|EloquentBuilder<*>|ExpressionContract|string $column + * @param Closure|self|EloquentBuilder<*>|Relation<*, *, *>|ExpressionContract|string $column * @param 'asc'|'desc'|SortDirection $direction * * @throws InvalidArgumentException */ - public function orderBy(Closure|self|EloquentBuilder|ExpressionContract|string $column, SortDirection|string $direction = SortDirection::Ascending): static + public function orderBy(Closure|self|EloquentBuilder|Relation|ExpressionContract|string $column, SortDirection|string $direction = SortDirection::Ascending): static { if ($this->isQueryable($column)) { [$query, $bindings] = $this->createSub($column); @@ -2432,9 +2460,9 @@ public function orderBy(Closure|self|EloquentBuilder|ExpressionContract|string $ /** * Add a descending "order by" clause to the query. * - * @param Closure|self|EloquentBuilder<*>|ExpressionContract|string $column + * @param Closure|self|EloquentBuilder<*>|Relation<*, *, *>|ExpressionContract|string $column */ - public function orderByDesc(Closure|self|EloquentBuilder|ExpressionContract|string $column): static + public function orderByDesc(Closure|self|EloquentBuilder|Relation|ExpressionContract|string $column): static { return $this->orderBy($column, SortDirection::Descending); } @@ -2442,7 +2470,7 @@ public function orderByDesc(Closure|self|EloquentBuilder|ExpressionContract|stri /** * Add an "order by" clause for a timestamp to the query. */ - public function latest(Closure|self|ExpressionContract|string $column = 'created_at'): static + public function latest(Closure|self|EloquentBuilder|Relation|ExpressionContract|string $column = 'created_at'): static { return $this->orderBy($column, SortDirection::Descending); } @@ -2450,7 +2478,7 @@ public function latest(Closure|self|ExpressionContract|string $column = 'created /** * Add an "order by" clause for a timestamp to the query. */ - public function oldest(Closure|self|ExpressionContract|string $column = 'created_at'): static + public function oldest(Closure|self|EloquentBuilder|Relation|ExpressionContract|string $column = 'created_at'): static { return $this->orderBy($column, SortDirection::Ascending); } @@ -2640,7 +2668,7 @@ public function forPageAfterId(int $perPage = 15, string|int|null $lastId = 0, s * * @param 'asc'|'desc'|SortDirection $direction */ - public function reorder(Closure|self|ExpressionContract|string|null $column = null, SortDirection|string $direction = SortDirection::Ascending): static + public function reorder(Closure|self|EloquentBuilder|Relation|ExpressionContract|string|null $column = null, SortDirection|string $direction = SortDirection::Ascending): static { $this->orders = null; $this->unionOrders = null; @@ -2657,7 +2685,7 @@ public function reorder(Closure|self|ExpressionContract|string|null $column = nu /** * Add descending "reorder" clause to the query. */ - public function reorderDesc(Closure|self|ExpressionContract|string|null $column): static + public function reorderDesc(Closure|self|EloquentBuilder|Relation|ExpressionContract|string|null $column): static { return $this->reorder($column, SortDirection::Descending); } @@ -2677,6 +2705,8 @@ protected function removeExistingOrdersFor(string $column): array * Add a "union" statement to the query. * * @param Closure|self|EloquentBuilder<*> $query + * + * @throws InvalidArgumentException */ public function union(Closure|self|EloquentBuilder $query, bool $all = false): static { @@ -2684,6 +2714,12 @@ public function union(Closure|self|EloquentBuilder $query, bool $all = false): s $query($query = $this->newQuery()); } + if ($query instanceof EloquentBuilder) { + $query = $query->toBase(); + } + + $this->assertNoTimeoutOnEmbeddedQuery($query); + $this->unions[] = compact('query', 'all'); $this->addBinding($query->getBindings(), 'union'); @@ -2695,6 +2731,8 @@ public function union(Closure|self|EloquentBuilder $query, bool $all = false): s * Add a "union all" statement to the query. * * @param Closure|self|EloquentBuilder<*> $query + * + * @throws InvalidArgumentException */ public function unionAll(Closure|self|EloquentBuilder $query): static { @@ -3058,12 +3096,17 @@ protected function runPaginationCountQuery(array $columns = ['*']): array { if ($this->groups || $this->havings) { $clone = $this->cloneForPaginationCount(); + $countQuery = $this->newQuery(); + + // The clone becomes an inner derived table, so its timeout belongs on the executed count statement. + $countQuery->timeout = $clone->timeout; + $clone->timeout = null; if (is_null($clone->columns) && ! empty($this->joins)) { $clone->select($this->from . '.*'); } - return $this->newQuery() + return $countQuery ->from(new Expression('(' . $clone->toSql() . ') as ' . $this->grammar->wrap('aggregate_table'))) ->mergeBindings($clone) ->setAggregate('count', $this->withoutSelectAliases($columns)) @@ -3566,9 +3609,9 @@ public function insertGetId(array $values, ?string $sequence = null): int|string /** * Insert new records into the table using a subquery. * - * @param Closure|self|EloquentBuilder<*>|string $query + * @param Closure|self|EloquentBuilder<*>|Relation<*, *, *>|string $query */ - public function insertUsing(array $columns, Closure|self|EloquentBuilder|string $query): int + public function insertUsing(array $columns, Closure|self|EloquentBuilder|Relation|string $query): int { $this->applyBeforeQueryCallbacks(); @@ -3583,9 +3626,9 @@ public function insertUsing(array $columns, Closure|self|EloquentBuilder|string /** * Insert new records into the table using a subquery while ignoring errors. * - * @param Closure|self|EloquentBuilder<*>|string $query + * @param Closure|self|EloquentBuilder<*>|Relation<*, *, *>|string $query */ - public function insertOrIgnoreUsing(array $columns, Closure|self|EloquentBuilder|string $query): int + public function insertOrIgnoreUsing(array $columns, Closure|self|EloquentBuilder|Relation|string $query): int { $this->applyBeforeQueryCallbacks(); @@ -4083,6 +4126,20 @@ protected function isQueryable(mixed $value): bool || $value instanceof Closure; } + /** + * Ensure an embedded query does not carry a statement-level timeout. + * + * @throws InvalidArgumentException + */ + protected function assertNoTimeoutOnEmbeddedQuery(self $query): void + { + if ($query->timeout !== null) { + throw new InvalidArgumentException( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.' + ); + } + } + /** * Clone the query. */ diff --git a/src/database/src/Query/Grammars/Grammar.php b/src/database/src/Query/Grammars/Grammar.php index 5593e3c9b..90e7064fe 100755 --- a/src/database/src/Query/Grammars/Grammar.php +++ b/src/database/src/Query/Grammars/Grammar.php @@ -56,6 +56,21 @@ class Grammar extends BaseGrammar * Compile a select query into SQL. */ public function compileSelect(Builder $query): string + { + return $this->compileSelectTimeout( + $query, + $this->compileSelectQuery($query), + ); + } + + /** + * Compile a select query without statement-level decoration. + * + * Embedded fragments are assembled by their builder's grammar because connection-owned details + * such as table prefixes resolve through that grammar. They bypass compileSelect because only a + * complete executed statement carries statement-level decoration. + */ + protected function compileSelectQuery(Builder $query): string { if (($query->unions || $query->havings) && $query->aggregate) { return $this->compileUnionAggregate($query); @@ -99,6 +114,14 @@ public function compileSelect(Builder $query): string return $sql; } + /** + * Compile the query timeout for a complete select statement. + */ + protected function compileSelectTimeout(Builder $query, string $sql): string + { + return $sql; + } + /** * Compile the components necessary for a select clause. */ @@ -485,7 +508,8 @@ protected function whereNested(Builder $query, array $where): string */ protected function whereSub(Builder $query, array $where): string { - $select = $this->compileSelect($where['query']); + $subquery = $where['query']; + $select = $subquery->getGrammar()->compileSelectQuery($subquery); return $this->wrap($where['column']) . ' ' . $where['operator'] . " ({$select})"; } @@ -495,7 +519,9 @@ protected function whereSub(Builder $query, array $where): string */ protected function whereExists(Builder $query, array $where): string { - return 'exists (' . $this->compileSelect($where['query']) . ')'; + $subquery = $where['query']; + + return 'exists (' . $subquery->getGrammar()->compileSelectQuery($subquery) . ')'; } /** @@ -503,7 +529,9 @@ protected function whereExists(Builder $query, array $where): string */ protected function whereNotExists(Builder $query, array $where): string { - return 'not exists (' . $this->compileSelect($where['query']) . ')'; + $subquery = $where['query']; + + return 'not exists (' . $subquery->getGrammar()->compileSelectQuery($subquery) . ')'; } /** @@ -923,8 +951,11 @@ protected function compileUnions(Builder $query): string protected function compileUnion(array $union): string { $conjunction = $union['all'] ? ' union all ' : ' union '; + $query = $union['query']; - return $conjunction . $this->wrapUnion($union['query']->toSql()); + return $conjunction . $this->wrapUnion( + $query->getGrammar()->compileSelectQuery($query) + ); } /** @@ -944,7 +975,7 @@ protected function compileUnionAggregate(Builder $query): string $query->aggregate = null; - return $sql . ' from (' . $this->compileSelect($query) . ') as ' . $this->wrapTable('temp_table'); + return $sql . ' from (' . $this->compileSelectQuery($query) . ') as ' . $this->wrapTable('temp_table'); } /** @@ -952,9 +983,12 @@ protected function compileUnionAggregate(Builder $query): string */ public function compileExists(Builder $query): string { - $select = $this->compileSelect($query); + $select = $this->compileSelectQuery($query); - return "select exists({$select}) as {$this->wrap('exists')}"; + return $this->compileSelectTimeout( + $query, + "select exists({$select}) as {$this->wrap('exists')}", + ); } /** diff --git a/src/database/src/Query/Grammars/MariaDbGrammar.php b/src/database/src/Query/Grammars/MariaDbGrammar.php index cc628c9cc..193ad00f6 100755 --- a/src/database/src/Query/Grammars/MariaDbGrammar.php +++ b/src/database/src/Query/Grammars/MariaDbGrammar.php @@ -6,10 +6,22 @@ use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\JoinLateralClause; +use Override; use RuntimeException; class MariaDbGrammar extends MySqlGrammar { + /** + * Compile the query timeout for a complete select statement. + */ + #[Override] + protected function compileSelectTimeout(Builder $query, string $sql): string + { + return $query->timeout === null + ? $sql + : 'SET STATEMENT max_statement_time=' . $query->timeout . ' FOR ' . $sql; + } + /** * Compile a "lateral join" clause. */ diff --git a/src/database/src/Query/Grammars/MySqlGrammar.php b/src/database/src/Query/Grammars/MySqlGrammar.php index 09ac0a6c4..83d3921b5 100755 --- a/src/database/src/Query/Grammars/MySqlGrammar.php +++ b/src/database/src/Query/Grammars/MySqlGrammar.php @@ -22,12 +22,11 @@ class MySqlGrammar extends Grammar protected array $operators = ['sounds like']; /** - * Compile a select query into SQL. + * Compile the query timeout for a complete select statement. */ - public function compileSelect(Builder $query): string + #[Override] + protected function compileSelectTimeout(Builder $query, string $sql): string { - $sql = parent::compileSelect($query); - if ($query->timeout === null) { return $sql; } @@ -35,8 +34,8 @@ public function compileSelect(Builder $query): string $milliseconds = $query->timeout * 1000; return preg_replace( - '/^select\b/i', - 'select /*+ MAX_EXECUTION_TIME(' . $milliseconds . ') */', + '/^(\(*)select\b/i', + '${1}select /*+ MAX_EXECUTION_TIME(' . $milliseconds . ') */', $sql, 1 ); diff --git a/src/database/src/Query/Grammars/PostgresGrammar.php b/src/database/src/Query/Grammars/PostgresGrammar.php index 35cb9ba4e..597a3771f 100755 --- a/src/database/src/Query/Grammars/PostgresGrammar.php +++ b/src/database/src/Query/Grammars/PostgresGrammar.php @@ -519,7 +519,7 @@ protected function compileUpdateWithJoinsOrLimit(Builder $query, array $values): $alias = last(preg_split('/\s+as\s+/i', $query->from)); - $selectSql = $this->compileSelect($query->select($alias . '.ctid')); + $selectSql = $this->compileSelectQuery($query->select($alias . '.ctid')); return "update {$table} set {$columns} where {$this->wrap('ctid')} in ({$selectSql})"; } @@ -566,7 +566,7 @@ protected function compileDeleteWithJoinsOrLimit(Builder $query): string $alias = last(preg_split('/\s+as\s+/i', $query->from)); - $selectSql = $this->compileSelect($query->select($alias . '.ctid')); + $selectSql = $this->compileSelectQuery($query->select($alias . '.ctid')); return "delete from {$table} where {$this->wrap('ctid')} in ({$selectSql})"; } diff --git a/src/database/src/Query/Grammars/SQLiteGrammar.php b/src/database/src/Query/Grammars/SQLiteGrammar.php index e64e86a88..f42acf39f 100755 --- a/src/database/src/Query/Grammars/SQLiteGrammar.php +++ b/src/database/src/Query/Grammars/SQLiteGrammar.php @@ -197,22 +197,6 @@ protected function compileJsonContainsKey(string $column): string return 'json_type(' . $field . $path . ') is not null'; } - /** - * Compile a group limit clause. - */ - protected function compileGroupLimit(Builder $query): string - { - $version = $query->getConnection()->getServerVersion(); - - if (version_compare($version, '3.25.0', '>=')) { - return parent::compileGroupLimit($query); - } - - $query->groupLimit = null; - - return $this->compileSelect($query); - } - /** * Compile an update statement into SQL. */ @@ -327,7 +311,7 @@ protected function compileUpdateWithJoinsOrLimit(Builder $query, array $values): $alias = last(preg_split('/\s+as\s+/i', $query->from)); - $selectSql = $this->compileSelect($query->select($alias . '.rowid')); + $selectSql = $this->compileSelectQuery($query->select($alias . '.rowid')); return "update {$table} set {$columns} where {$this->wrap('rowid')} in ({$selectSql})"; } @@ -376,7 +360,7 @@ protected function compileDeleteWithJoinsOrLimit(Builder $query): string $alias = last(preg_split('/\s+as\s+/i', $query->from)); - $selectSql = $this->compileSelect($query->select($alias . '.rowid')); + $selectSql = $this->compileSelectQuery($query->select($alias . '.rowid')); return "delete from {$table} where {$this->wrap('rowid')} in ({$selectSql})"; } diff --git a/tests/Database/DatabaseMariaDbQueryGrammarTest.php b/tests/Database/DatabaseMariaDbQueryGrammarTest.php index 89c9cd818..9621d18a3 100755 --- a/tests/Database/DatabaseMariaDbQueryGrammarTest.php +++ b/tests/Database/DatabaseMariaDbQueryGrammarTest.php @@ -5,7 +5,9 @@ namespace Hypervel\Tests\Database; use Hypervel\Database\Connection; +use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\Grammars\MariaDbGrammar; +use Hypervel\Database\Query\Processors\Processor; use Hypervel\Tests\TestCase; use JsonException; use Mockery as m; @@ -20,7 +22,7 @@ public function testUpdateBindingsRejectUnencodableArrays(): void ->prepareBindingsForUpdate([], ['payload' => [NAN]]); } - public function testToRawSql() + public function testToRawSql(): void { $connection = m::mock(Connection::class); $connection->shouldReceive('escape')->with('foo', false)->andReturn("'foo'"); @@ -33,4 +35,82 @@ public function testToRawSql() $this->assertSame('select * from "users" where \'Hello\\\'World?\' IS NOT NULL AND "email" = \'foo\'', $query); } + + public function testTimeoutDecoratesSimpleAggregateAndLockingStatements(): void + { + $simple = $this->getBuilder()->from('users')->where('active', true)->timeout(5); + $this->assertSame( + 'SET STATEMENT max_statement_time=5 FOR select * from `users` where `active` = ?', + $simple->toSql() + ); + + $aggregate = $this->getBuilder()->from('users')->timeout(4); + $aggregate->aggregate = ['function' => 'count', 'columns' => ['*']]; + $this->assertSame( + 'SET STATEMENT max_statement_time=4 FOR select count(*) as `aggregate` from `users`', + $aggregate->toSql() + ); + + $locking = $this->getBuilder()->from('users')->where('id', 1)->lockForUpdate()->timeout(3); + $this->assertSame( + 'SET STATEMENT max_statement_time=3 FOR select * from `users` where `id` = ? for update', + $locking->toSql() + ); + } + + public function testTimeoutDecoratesOrdinaryAndAggregateUnionStatementsOnce(): void + { + $builder = $this->getBuilder() + ->from('posts') + ->where('published', true) + ->unionAll($this->getBuilder()->from('videos')->where('published', false)) + ->timeout(5); + + $this->assertSame( + 'SET STATEMENT max_statement_time=5 FOR (select * from `posts` where `published` = ?) union all (select * from `videos` where `published` = ?)', + $builder->toSql() + ); + + $builder->aggregate = ['function' => 'count', 'columns' => ['*']]; + + $sql = $builder->toSql(); + + $this->assertSame( + 'SET STATEMENT max_statement_time=5 FOR select count(*) as `aggregate` from ((select * from `posts` where `published` = ?) union all (select * from `videos` where `published` = ?)) as `temp_table`', + $sql + ); + $this->assertSame(1, substr_count($sql, 'SET STATEMENT')); + $this->assertSame([true, false], $builder->getBindings()); + } + + public function testTimeoutDecoratesExistsAtTheStatementRoot(): void + { + $builder = $this->getBuilder()->from('users')->where('active', true)->timeout(4); + + $this->assertSame( + 'SET STATEMENT max_statement_time=4 FOR select exists(select * from `users` where `active` = ?) as `exists`', + $builder->getGrammar()->compileExists($builder) + ); + } + + public function testTimeoutCanBeCleared(): void + { + $builder = $this->getBuilder(); + $builder->select('*')->from('users')->timeout(60)->timeout(null); + + $this->assertSame('select * from `users`', $builder->toSql()); + } + + protected function getBuilder(): Builder + { + $connection = m::mock(Connection::class); + $connection->shouldReceive('getDatabaseName')->andReturn('database'); + $connection->shouldReceive('getTablePrefix')->andReturn(''); + + return new Builder( + $connection, + new MariaDbGrammar($connection), + m::mock(Processor::class) + ); + } } diff --git a/tests/Database/DatabaseMySqlQueryGrammarTest.php b/tests/Database/DatabaseMySqlQueryGrammarTest.php index 8872279cc..1c691eced 100755 --- a/tests/Database/DatabaseMySqlQueryGrammarTest.php +++ b/tests/Database/DatabaseMySqlQueryGrammarTest.php @@ -23,7 +23,7 @@ public function testUpdateBindingsRejectUnencodableArrays(): void ->prepareBindingsForUpdate([], ['payload' => [NAN]]); } - public function testToRawSql() + public function testToRawSql(): void { $connection = m::mock(Connection::class); $connection->shouldReceive('escape')->with('foo', false)->andReturn("'foo'"); @@ -66,6 +66,74 @@ public function testTimeoutWithDistinctAndAggregateQueries(): void ); } + public function testTimeoutDecoratesOrdinaryAndAggregateUnionStatementsOnce(): void + { + $builder = $this->getBuilder() + ->select('*') + ->from('posts') + ->where('published', true) + ->unionAll( + $this->getBuilder() + ->select('*') + ->from('videos') + ->where('published', false) + ) + ->timeout(5); + + $this->assertSame( + '(select /*+ MAX_EXECUTION_TIME(5000) */ * from `posts` where `published` = ?) union all (select * from `videos` where `published` = ?)', + $builder->toSql() + ); + $this->assertSame([true, false], $builder->getBindings()); + + $builder->aggregate = ['function' => 'count', 'columns' => ['*']]; + + $sql = $builder->toSql(); + + $this->assertSame( + 'select /*+ MAX_EXECUTION_TIME(5000) */ count(*) as `aggregate` from ((select * from `posts` where `published` = ?) union all (select * from `videos` where `published` = ?)) as `temp_table`', + $sql + ); + $this->assertSame(1, substr_count($sql, 'MAX_EXECUTION_TIME')); + } + + public function testTimeoutDecoratesExistsAndLockingStatementsAtTheRoot(): void + { + $exists = $this->getBuilder()->from('users')->where('active', true)->timeout(4); + + $this->assertSame( + 'select /*+ MAX_EXECUTION_TIME(4000) */ exists(select * from `users` where `active` = ?) as `exists`', + $exists->getGrammar()->compileExists($exists) + ); + + $locking = $this->getBuilder()->from('users')->where('id', 1)->lockForUpdate()->timeout(3); + + $this->assertSame( + 'select /*+ MAX_EXECUTION_TIME(3000) */ * from `users` where `id` = ? for update', + $locking->toSql() + ); + } + + public function testOuterTimeoutLeavesAnUntimedSubqueryUndecorated(): void + { + $subquery = $this->getBuilder()->select('user_id')->from('memberships')->where('active', true); + $builder = $this->getBuilder() + ->select('*') + ->from('users') + ->whereIn('id', $subquery) + ->where('status', 'enabled') + ->timeout(6); + + $sql = $builder->toSql(); + + $this->assertSame( + 'select /*+ MAX_EXECUTION_TIME(6000) */ * from `users` where `id` in (select `user_id` from `memberships` where `active` = ?) and `status` = ?', + $sql + ); + $this->assertSame([true, 'enabled'], $builder->getBindings()); + $this->assertSame(1, substr_count($sql, 'MAX_EXECUTION_TIME')); + } + public function testTimeoutCanBeCleared(): void { $builder = $this->getBuilder(); diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index f13e8a53f..73adf77ba 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -12,6 +12,8 @@ use Hypervel\Contracts\Database\Query\ConditionExpression; use Hypervel\Database\Connection; use Hypervel\Database\Eloquent\Builder as EloquentBuilder; +use Hypervel\Database\Eloquent\Model; +use Hypervel\Database\Eloquent\Relations\HasMany; use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\Expression as Raw; use Hypervel\Database\Query\Grammars\Grammar; @@ -2582,6 +2584,485 @@ public function testGetCountForPaginationWithUnionLimitAndOffset() $this->assertEquals(1, $count); } + public function testGroupLimitCompilationWithAndWithoutOffset(): void + { + $builder = $this->getSQLiteBuilder() + ->select('id') + ->from('posts') + ->where('active', true) + ->orderByDesc('created_at') + ->groupLimit(2, 'user_id'); + + $this->assertSame( + 'select * from (select "id", row_number() over (partition by "user_id" order by "created_at" desc) as "hypervel_row" from "posts" where "active" = ?) as "hypervel_table" where "hypervel_row" <= 2 order by "hypervel_row"', + $builder->toSql() + ); + $this->assertSame([true], $builder->getBindings()); + + $offsetBuilder = $this->getSQLiteBuilder() + ->select('id') + ->from('posts') + ->where('active', true) + ->orderByDesc('created_at') + ->offset(3) + ->groupLimit(2, 'user_id'); + + $this->assertSame( + 'select * from (select "id", row_number() over (partition by "user_id" order by "created_at" desc) as "hypervel_row" from "posts" where "active" = ?) as "hypervel_table" where "hypervel_row" <= 5 and "hypervel_row" > 3 order by "hypervel_row"', + $offsetBuilder->toSql() + ); + $this->assertSame([true], $offsetBuilder->getBindings()); + } + + public function testGroupedPaginationAppliesTimeoutToTheOuterCountStatement(): void + { + $builder = $this->getMySqlBuilder() + ->from('users') + ->where('active', true) + ->groupBy('team_id') + ->having('score', '>', 10) + ->timeout(5); + + $builder->getConnection()->shouldReceive('select')->once()->with( + 'select /*+ MAX_EXECUTION_TIME(5000) */ count(*) as `aggregate` from (select * from `users` where `active` = ? group by `team_id` having `score` > ?) as `aggregate_table`', + [true, 10], + true, + [], + )->andReturn([['aggregate' => 1]]); + $builder->getProcessor()->shouldReceive('processSelect')->once()->andReturnUsing(function ($builder, $results) { + return $results; + }); + + $this->assertSame(1, $builder->getCountForPagination()); + $this->assertSame(5, $builder->timeout); + $this->assertSame( + 'select /*+ MAX_EXECUTION_TIME(5000) */ * from `users` where `active` = ? group by `team_id` having `score` > ?', + $builder->toSql() + ); + } + + public function testOuterTimeoutAcceptsUntimedSubqueriesExistsClausesAndUnionMembers(): void + { + $parsed = $this->getMySqlBuilder()->select('user_id')->from('memberships')->where('active', true); + $exists = $this->getMySqlBuilder()->selectRaw('1')->from('flags')->whereColumn('flags.user_id', 'users.id'); + $union = $this->getMySqlBuilder()->from('archived_users')->where('status', 'enabled'); + + $builder = $this->getMySqlBuilder() + ->from('users') + ->whereIn('id', $parsed) + ->whereExists($exists) + ->unionAll($union) + ->timeout(6); + + $sql = $builder->toSql(); + + $this->assertSame( + '(select /*+ MAX_EXECUTION_TIME(6000) */ * from `users` where `id` in (select `user_id` from `memberships` where `active` = ?) and exists (select 1 from `flags` where `flags`.`user_id` = `users`.`id`)) union all (select * from `archived_users` where `status` = ?)', + $sql + ); + $this->assertSame([true, 'enabled'], $builder->getBindings()); + $this->assertSame(1, substr_count($sql, 'MAX_EXECUTION_TIME')); + } + + public function testRetainedMySqlSubqueriesUseTheOuterStatementTimeoutAfterTheirOwnTimeoutChanges(): void + { + $this->assertRetainedSubqueriesUseOuterStatementTimeout( + fn () => $this->getMySqlBuilder(), + '(select /*+ MAX_EXECUTION_TIME(5000) */ * from `users` where `score` > (select `score` from `scores` where `active` = ?) and exists (select 1 from `flags` where `active` = ?)) union all (select * from `archived_users` where `active` = ?)', + 'select /*+ MAX_EXECUTION_TIME(2000) */ `score` from `scores` where `active` = ?', + 'select /*+ MAX_EXECUTION_TIME(2000) */ 1 from `flags` where `active` = ?', + 'select /*+ MAX_EXECUTION_TIME(2000) */ * from `archived_users` where `active` = ?', + ); + } + + public function testRetainedMariaDbSubqueriesUseTheOuterStatementTimeoutAfterTheirOwnTimeoutChanges(): void + { + $this->assertRetainedSubqueriesUseOuterStatementTimeout( + fn () => $this->getMariaDbBuilder(), + 'SET STATEMENT max_statement_time=5 FOR (select * from `users` where `score` > (select `score` from `scores` where `active` = ?) and exists (select 1 from `flags` where `active` = ?)) union all (select * from `archived_users` where `active` = ?)', + 'SET STATEMENT max_statement_time=2 FOR select `score` from `scores` where `active` = ?', + 'SET STATEMENT max_statement_time=2 FOR select 1 from `flags` where `active` = ?', + 'SET STATEMENT max_statement_time=2 FOR select * from `archived_users` where `active` = ?', + ); + } + + public function testRetainedSubqueriesUseTheirOwnGrammarTablePrefix(): void + { + $whereSubquery = $this->getBuilder('child_') + ->select('score') + ->from('scores') + ->where('active', true); + $where = $this->getBuilder() + ->from('users') + ->where('score', '>', $whereSubquery); + + $this->assertSame( + 'select * from "users" where "score" > (select "score" from "child_scores" where "active" = ?)', + $where->toSql() + ); + $this->assertSame([true], $where->getBindings()); + + $existsSubquery = $this->getBuilder('child_')->from('flags')->where('active', true); + $exists = $this->getBuilder()->from('users')->whereExists($existsSubquery); + + $this->assertSame( + 'select * from "users" where exists (select * from "child_flags" where "active" = ?)', + $exists->toSql() + ); + $this->assertSame([true], $exists->getBindings()); + + $notExistsSubquery = $this->getBuilder('child_')->from('blocks')->where('active', true); + $notExists = $this->getBuilder()->from('users')->whereNotExists($notExistsSubquery); + + $this->assertSame( + 'select * from "users" where not exists (select * from "child_blocks" where "active" = ?)', + $notExists->toSql() + ); + $this->assertSame([true], $notExists->getBindings()); + + $unionMember = $this->getBuilder('child_')->from('archived_users')->where('active', true); + $union = $this->getBuilder()->from('users')->unionAll($unionMember); + + $this->assertSame( + '(select * from "users") union all (select * from "child_archived_users" where "active" = ?)', + $union->toSql() + ); + $this->assertSame([true], $union->getBindings()); + } + + public function testEloquentUnionMemberIsNormalizedWithItsGlobalScope(): void + { + $union = new EloquentBuilder($this->getMySqlBuilder()); + $union->setModel(new QueryableSubqueryRelatedModel); + $union->withGlobalScope('active', fn (EloquentBuilder $query) => $query->where('active', true)); + + $builder = $this->getMySqlBuilder() + ->from('users') + ->where('tenant_id', 7) + ->unionAll($union); + + $this->assertSame( + '(select * from `users` where `tenant_id` = ?) union all (select * from `queryable_subquery_related` where (`active` = ?))', + $builder->toSql() + ); + $this->assertSame([7, true], $builder->getBindings()); + } + + public function testParsedEloquentSubqueryRejectsATimeoutAppliedByItsGlobalScope(): void + { + $scopeApplications = 0; + $embedded = new EloquentBuilder($this->getMySqlBuilder()); + $embedded->setModel(new QueryableSubqueryRelatedModel); + $embedded->withGlobalScope('timeout', function (EloquentBuilder $query) use (&$scopeApplications): void { + ++$scopeApplications; + $query->timeout(2); + }); + + try { + $this->getMySqlBuilder()->from('users')->whereIn('id', $embedded); + $this->fail('Expected the embedded query timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame(1, $scopeApplications); + } + + public function testEloquentUnionMemberRejectsATimeoutAppliedByItsGlobalScope(): void + { + $scopeApplications = 0; + $embedded = new EloquentBuilder($this->getMySqlBuilder()); + $embedded->setModel(new QueryableSubqueryRelatedModel); + $embedded->withGlobalScope('timeout', function (EloquentBuilder $query) use (&$scopeApplications): void { + ++$scopeApplications; + $query->timeout(2); + }); + + try { + $this->getMySqlBuilder()->from('users')->unionAll($embedded); + $this->fail('Expected the embedded query timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame(1, $scopeApplications); + } + + public function testParsedSubqueryRejectsItsOwnTimeoutBeforeEmbeddingIt(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $embedded = $this->getMySqlBuilder()->select('user_id')->from('memberships')->where('active', true)->timeout(2); + + $this->assertTimedEmbeddingRejectedBeforeQueryIsEmbedded( + $outer, + $embedded, + fn () => $outer->whereIn('id', $embedded), + ); + } + + public function testParsedRelationRejectsItsOwnTimeoutBeforeEmbeddingIt(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $relation = $this->getRelationSubquery(grammarClass: MySqlGrammar::class); + $relation->timeout(2); + $sql = $outer->toSql(); + $bindings = $outer->getBindings(); + + try { + $outer->update(['score' => $relation]); + $this->fail('Expected the embedded relation timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $outer->toSql()); + $this->assertSame($bindings, $outer->getBindings()); + } + + public function testWhereSubqueryRejectsItsOwnTimeoutBeforeEmbeddingIt(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $embedded = $this->getMySqlBuilder()->select('score')->from('scores')->where('active', true)->timeout(2); + + $this->assertTimedEmbeddingRejectedBeforeQueryIsEmbedded( + $outer, + $embedded, + fn () => $outer->where('score', '>', $embedded), + ); + } + + public function testWhereExistsRejectsItsOwnTimeoutBeforeEmbeddingIt(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $embedded = $this->getMySqlBuilder()->selectRaw('1')->from('flags')->where('active', true)->timeout(2); + + $this->assertTimedEmbeddingRejectedBeforeQueryIsEmbedded( + $outer, + $embedded, + fn () => $outer->addWhereExistsQuery($embedded), + ); + } + + public function testUnionRejectsItsOwnTimeoutBeforeEmbeddingIt(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $embedded = $this->getMySqlBuilder()->from('archived_users')->where('active', true)->timeout(2); + + $this->assertTimedEmbeddingRejectedBeforeQueryIsEmbedded( + $outer, + $embedded, + fn () => $outer->unionAll($embedded), + ); + } + + public function testExplainRejectsQueryTimeoutWithoutMutatingTheBuilder(): void + { + $builder = $this->getMySqlBuilder()->from('users')->where('id', 1)->timeout(2); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $builder->explain(); + $this->fail('Expected the EXPLAIN timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A query timeout cannot be applied to an EXPLAIN statement. Clear the timeout before calling explain().', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + + public function testExplainKeepsUntimedStatementSqlAndBindings(): void + { + $builder = $this->getMySqlBuilder()->from('users')->where('id', 1); + $explanation = (object) ['id' => 1]; + + $builder->getConnection()->expects('select')->with( + 'EXPLAIN select * from `users` where `id` = ?', + [1], + )->andReturn([$explanation]); + + $this->assertSame([$explanation], $builder->explain()->all()); + } + + public function testRelationSubqueriesCompileInSelectAndFromClausesAcrossDatabases(): void + { + $select = $this->getBuilder() + ->from('parents') + ->select(['related_score' => $this->getRelationSubquery('other_database')]); + + $this->assertSame( + 'select (select "score" from "other_database"."queryable_subquery_related" where "queryable_subquery_related"."parent_id" = ? and "queryable_subquery_related"."parent_id" is not null) as "related_score" from "parents"', + $select->toSql() + ); + $this->assertSame([7], $select->getBindings()); + + $from = $this->getBuilder() + ->select('*') + ->from($this->getRelationSubquery('other_database'), 'related'); + + $this->assertSame( + 'select * from (select "score" from "other_database"."queryable_subquery_related" where "queryable_subquery_related"."parent_id" = ? and "queryable_subquery_related"."parent_id" is not null) as "related"', + $from->toSql() + ); + $this->assertSame([7], $from->getBindings()); + } + + public function testCrossDatabaseQualificationKeepsQualifiedDerivedTablesAndTablelessSubqueriesUnchanged(): void + { + $connection = m::mock(Connection::class); + $connection->shouldReceive('getDatabaseName')->andReturn('other_database'); + $connection->shouldReceive('getTablePrefix')->andReturn(''); + + $derived = new Builder($connection, new Grammar($connection), m::mock(Processor::class)); + $derived->from('other_database.events'); + + $subquery = new Builder($connection, new Grammar($connection), m::mock(Processor::class)); + $subquery->fromSub($derived, 'derived'); + + $outer = $this->getBuilder()->from('users')->whereIn('id', $subquery); + + $this->assertSame( + 'select * from "users" where "id" in (select * from (select * from "other_database"."events") as "derived")', + $outer->toSql() + ); + + $tableless = new Builder($connection, new Grammar($connection), m::mock(Processor::class)); + $tableless->selectRaw('1'); + + $this->assertSame( + 'select * from "users" where "id" in (select 1)', + $this->getBuilder()->from('users')->whereIn('id', $tableless)->toSql() + ); + } + + public function testRelationSubqueriesCompileInWhereAndStraightJoinClauses(): void + { + $where = $this->getBuilder() + ->from('parents') + ->where('score', '>', $this->getRelationSubquery()); + + $this->assertSame( + 'select * from "parents" where "score" > (select "score" from "queryable_subquery_related" where "queryable_subquery_related"."parent_id" = ? and "queryable_subquery_related"."parent_id" is not null)', + $where->toSql() + ); + $this->assertSame([7], $where->getBindings()); + + $join = $this->getMySqlBuilder() + ->from('parents') + ->straightJoinSub( + $this->getRelationSubquery(grammarClass: MySqlGrammar::class), + 'related', + 'related.parent_id', + '=', + 'parents.id', + ); + + $this->assertSame( + 'select * from `parents` straight_join (select `score` from `queryable_subquery_related` where `queryable_subquery_related`.`parent_id` = ? and `queryable_subquery_related`.`parent_id` is not null) as `related` on `related`.`parent_id` = `parents`.`id`', + $join->toSql() + ); + $this->assertSame([7], $join->getBindings()); + } + + public function testRelationSubqueryCompilesForInsertUsing(): void + { + $builder = $this->getBuilder()->from('archived_scores'); + $builder->getConnection()->expects('affectingStatement')->with( + 'insert into "archived_scores" ("score") select "score" from "queryable_subquery_related" where "queryable_subquery_related"."parent_id" = ? and "queryable_subquery_related"."parent_id" is not null', + [7], + )->andReturn(1); + + $this->assertSame( + 1, + $builder->insertUsing(['score'], $this->getRelationSubquery()) + ); + } + + public function testWhereForwardersAcceptQueryBuilderSubqueries(): void + { + $subquery = $this->getBuilder() + ->select('score') + ->from('scores') + ->where('active', true); + + $builder = $this->getBuilder() + ->from('parents') + ->where('tenant_id', 1) + ->orWhere($subquery, '>', 5) + ->whereNot($subquery, '<', 0) + ->orWhereNot($subquery, '=', 3); + + $this->assertSame( + 'select * from "parents" where "tenant_id" = ? or (select "score" from "scores" where "active" = ?) > ? and not (select "score" from "scores" where "active" = ?) < ? or not (select "score" from "scores" where "active" = ?) = ?', + $builder->toSql() + ); + $this->assertSame([1, true, 5, true, 0, true, 3], $builder->getBindings()); + } + + public function testBetweenForwardersAcceptQueryBuilderSubqueries(): void + { + $subquery = $this->getBuilder()->select('score')->from('scores')->where('active', true); + $builder = $this->getBuilder() + ->from('parents') + ->whereBetween($subquery, [1, 2]) + ->orWhereBetween($subquery, [3, 4]) + ->whereNotBetween($subquery, [5, 6]) + ->orWhereNotBetween($subquery, [7, 8]); + + $this->assertSame( + 'select * from "parents" where (select "score" from "scores" where "active" = ?) between ? and ? or (select "score" from "scores" where "active" = ?) between ? and ? and (select "score" from "scores" where "active" = ?) not between ? and ? or (select "score" from "scores" where "active" = ?) not between ? and ?', + $builder->toSql() + ); + $this->assertSame([true, 1, 2, true, 3, 4, true, 5, 6, true, 7, 8], $builder->getBindings()); + } + + public function testBetweenColumnsForwardersAcceptQueryBuilderSubqueries(): void + { + $subquery = $this->getBuilder()->select('score')->from('scores')->where('active', true); + $builder = $this->getBuilder() + ->from('parents') + ->whereBetweenColumns($subquery, ['minimum', 'maximum']) + ->orWhereBetweenColumns($subquery, ['minimum', 'maximum']) + ->whereNotBetweenColumns($subquery, ['minimum', 'maximum']) + ->orWhereNotBetweenColumns($subquery, ['minimum', 'maximum']); + + $this->assertSame( + 'select * from "parents" where (select "score" from "scores" where "active" = ?) between "minimum" and "maximum" or (select "score" from "scores" where "active" = ?) between "minimum" and "maximum" and (select "score" from "scores" where "active" = ?) not between "minimum" and "maximum" or (select "score" from "scores" where "active" = ?) not between "minimum" and "maximum"', + $builder->toSql() + ); + $this->assertSame([true, true, true, true], $builder->getBindings()); + } + + public function testOrderForwardersAcceptQueryableSubqueries(): void + { + $subquery = $this->getBuilder()->select('score')->from('scores')->where('active', true); + $sql = 'select * from "parents" order by (select "score" from "scores" where "active" = ?)'; + + foreach ([ + [$this->getBuilder()->from('parents')->orderByDesc($subquery), $sql . ' desc'], + [$this->getBuilder()->from('parents')->latest($subquery), $sql . ' desc'], + [$this->getBuilder()->from('parents')->oldest($subquery), $sql . ' asc'], + [$this->getBuilder()->from('parents')->orderBy('name')->reorder($subquery, 'desc'), $sql . ' desc'], + [$this->getBuilder()->from('parents')->orderBy('name')->reorderDesc($subquery), $sql . ' desc'], + ] as [$builder, $expectedSql]) { + $this->assertSame($expectedSql, $builder->toSql()); + $this->assertSame([true], $builder->getBindings()); + } + } + public function testWhereShortcut() { $builder = $this->getBuilder(); @@ -7148,6 +7629,99 @@ protected function getConnection(string $prefix = '') return $connection; } + /** + * Get a real relation backed by a query builder on the given database. + * + * @param class-string $grammarClass + */ + protected function getRelationSubquery(string $database = 'database', string $grammarClass = Grammar::class): HasMany + { + $connection = m::mock(Connection::class); + $connection->shouldReceive('getDatabaseName')->andReturn($database); + $connection->shouldReceive('getTablePrefix')->andReturn(''); + + $query = new EloquentBuilder(new Builder( + $connection, + new $grammarClass($connection), + m::mock(Processor::class), + )); + $query->setModel(new QueryableSubqueryRelatedModel); + + $parent = new QueryableSubqueryParentModel; + $parent->id = 7; + $parent->exists = true; + + $relation = new HasMany( + $query, + $parent, + 'queryable_subquery_related.parent_id', + 'id', + ); + $relation->select('score'); + + return $relation; + } + + /** + * Assert retained subqueries carry only the outer statement's timeout. + * + * @param Closure(): Builder $newBuilder + */ + protected function assertRetainedSubqueriesUseOuterStatementTimeout( + Closure $newBuilder, + string $outerSql, + string $whereSql, + string $existsSql, + string $unionSql, + ): void { + $where = $newBuilder()->select('score')->from('scores')->where('active', true); + $exists = $newBuilder()->selectRaw('1')->from('flags')->where('active', true); + $union = $newBuilder()->from('archived_users')->where('active', true); + + $outer = $newBuilder() + ->from('users') + ->where('score', '>', $where) + ->whereExists($exists) + ->unionAll($union) + ->timeout(5); + + $where->timeout(2); + $exists->timeout(2); + $union->timeout(2); + + $this->assertSame($outerSql, $outer->toSql()); + $this->assertSame($whereSql, $where->toSql()); + $this->assertSame($existsSql, $exists->toSql()); + $this->assertSame($unionSql, $union->toSql()); + $this->assertSame([true, true, true], $outer->getBindings()); + } + + /** + * Assert that a timed embedded query is rejected before it is embedded. + */ + protected function assertTimedEmbeddingRejectedBeforeQueryIsEmbedded(Builder $outer, Builder $embedded, Closure $accept): void + { + $outerSql = $outer->toSql(); + $outerBindings = $outer->getBindings(); + $embeddedSql = $embedded->toSql(); + $embeddedBindings = $embedded->getBindings(); + + try { + $accept(); + $this->fail('Expected the embedded query timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($outerSql, $outer->toSql()); + $this->assertSame($outerBindings, $outer->getBindings()); + $this->assertSame($embeddedSql, $embedded->toSql()); + $this->assertSame($embeddedBindings, $embedded->getBindings()); + } + protected function getBuilder(string $prefix = '') { $connection = $this->getConnection(prefix: $prefix); @@ -7223,3 +7797,17 @@ protected function getMockQueryBuilder() ])->makePartial(); } } + +class QueryableSubqueryParentModel extends Model +{ + protected ?string $table = 'queryable_subquery_parents'; + + public bool $timestamps = false; +} + +class QueryableSubqueryRelatedModel extends Model +{ + protected ?string $table = 'queryable_subquery_related'; + + public bool $timestamps = false; +} diff --git a/tests/Integration/Database/MariaDb/QueryTimeoutTest.php b/tests/Integration/Database/MariaDb/QueryTimeoutTest.php new file mode 100644 index 000000000..ae795e076 --- /dev/null +++ b/tests/Integration/Database/MariaDb/QueryTimeoutTest.php @@ -0,0 +1,22 @@ +make('config'); + $connection = $config->string('database.default'); + $config->set( + 'database.connections.query_timeout_peer', + $config->array('database.connections.' . $connection), + ); + } + + protected function afterRefreshingDatabase(): void + { + Schema::create('query_timeout_probes', function (Blueprint $table): void { + $table->increments('id'); + }); + + DB::table('query_timeout_probes')->insert(['id' => 1]); + } + + public function testTimeoutInterruptsOrdinarySelect(): void + { + $this->assertQueryTimesOut( + fn () => DB::table('query_timeout_probes')->selectRaw('SLEEP(2) as delay')->timeout(1)->get() + ); + } + + public function testTimeoutInterruptsExistsSelect(): void + { + $this->assertQueryTimesOut( + fn () => DB::table('query_timeout_probes') + ->whereRaw('SLEEP(2) = 0') + ->timeout(1) + ->exists() + ); + } + + public function testTimeoutInterruptsUnionSelect(): void + { + $this->assertQueryTimesOut( + fn () => DB::query() + ->selectRaw('1 as value') + ->unionAll(DB::query()->selectRaw('SLEEP(2) as value')) + ->timeout(1) + ->get() + ); + } + + public function testTimeoutInterruptsBlockedLockingSelect(): void + { + $owner = DB::connection(); + $contender = DB::connection('query_timeout_peer'); + $originalLockWaitTimeout = $contender->scalar('select @@session.innodb_lock_wait_timeout'); + + try { + $contender->statement('SET SESSION innodb_lock_wait_timeout = 3'); + $owner->beginTransaction(); + $owner->table('query_timeout_probes')->where('id', 1)->lockForUpdate()->first(); + + $this->assertQueryTimesOut( + fn () => $contender->table('query_timeout_probes') + ->where('id', 1) + ->lockForUpdate() + ->timeout(1) + ->first() + ); + } finally { + if ($owner->transactionLevel() > 0) { + $owner->rollBack(); + } + + $contender->statement( + 'SET SESSION innodb_lock_wait_timeout = ' . (int) $originalLockWaitTimeout + ); + } + } + + /** + * Assert that the database reports its statement-timeout error. + */ + protected function assertQueryTimesOut(Closure $callback): void + { + try { + $callback(); + $this->fail('Expected the database statement timeout to interrupt the query.'); + } catch (QueryException $exception) { + $this->assertMatchesRegularExpression($this->timeoutErrorPattern(), $exception->getMessage()); + } + } + + /** + * Get the database-specific timeout error pattern. + */ + abstract protected function timeoutErrorPattern(): string; +} From df22337c4e657360e93a30f39051fef8b55c1166 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:18:54 +0000 Subject: [PATCH 3/7] fix(database): align Eloquent embedded query boundaries Widen Eloquent forwarding methods to the Query, Eloquent, and Relation inputs their Query Builder callees already support, preserving Laravel-style application-facing query composition. Validate relationship constraint timeouts after scope and relation constraints are merged but before the child query is stored or its bindings are added. Apply one relationship-specific diagnostic across withAggregate, withExists, and both whereHas execution strategies. Add family-level coverage for the widened forwarding APIs and regressions for aggregate, exists, and count constraint timeout rejection. --- src/database/src/Eloquent/Builder.php | 24 +-- .../Concerns/QueriesRelationships.php | 31 +++- .../Database/DatabaseEloquentBuilderTest.php | 160 ++++++++++++++++++ 3 files changed, 197 insertions(+), 18 deletions(-) diff --git a/src/database/src/Eloquent/Builder.php b/src/database/src/Eloquent/Builder.php index 92ae925d6..11172d74c 100644 --- a/src/database/src/Eloquent/Builder.php +++ b/src/database/src/Eloquent/Builder.php @@ -312,9 +312,9 @@ public function except(mixed $models): static /** * Add a basic where clause to the query. * - * @param array|(Closure(static): mixed)|Expression|string $column + * @param array|(Closure(static): mixed)|self|QueryBuilder|Relation<*, *, *>|Expression|string $column */ - public function where(array|Closure|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static + public function where(array|Closure|self|QueryBuilder|Relation|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { if ($column instanceof Closure && is_null($operator)) { // @phpstan-ignore argument.type (closure receives Builder instance, static type not required) @@ -336,10 +336,10 @@ public function where(array|Closure|Expression|string $column, mixed $operator = /** * Add a basic where clause to the query, and return the first result. * - * @param array|(Closure(static): mixed)|Expression|string $column + * @param array|(Closure(static): mixed)|self|QueryBuilder|Relation<*, *, *>|Expression|string $column * @return null|TModel */ - public function firstWhere(array|Closure|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): ?Model + public function firstWhere(array|Closure|self|QueryBuilder|Relation|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): ?Model { return $this->where(...func_get_args())->first(); } @@ -347,9 +347,9 @@ public function firstWhere(array|Closure|Expression|string $column, mixed $opera /** * Add an "or where" clause to the query. * - * @param array|(Closure(static): mixed)|Expression|string $column + * @param array|(Closure(static): mixed)|self|QueryBuilder|Relation<*, *, *>|Expression|string $column */ - public function orWhere(array|Closure|Expression|string $column, mixed $operator = null, mixed $value = null): static + public function orWhere(array|Closure|self|QueryBuilder|Relation|Expression|string $column, mixed $operator = null, mixed $value = null): static { [$value, $operator] = $this->query->prepareValueAndOperator( $value, @@ -363,9 +363,9 @@ public function orWhere(array|Closure|Expression|string $column, mixed $operator /** * Add a basic "where not" clause to the query. * - * @param array|(Closure(static): mixed)|Expression|string $column + * @param array|(Closure(static): mixed)|self|QueryBuilder|Relation<*, *, *>|Expression|string $column */ - public function whereNot(array|Closure|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static + public function whereNot(array|Closure|self|QueryBuilder|Relation|Expression|string $column, mixed $operator = null, mixed $value = null, string $boolean = 'and'): static { return $this->where($column, $operator, $value, $boolean . ' not'); } @@ -373,9 +373,9 @@ public function whereNot(array|Closure|Expression|string $column, mixed $operato /** * Add an "or where not" clause to the query. * - * @param array|(Closure(static): mixed)|Expression|string $column + * @param array|(Closure(static): mixed)|self|QueryBuilder|Relation<*, *, *>|Expression|string $column */ - public function orWhereNot(array|Closure|Expression|string $column, mixed $operator = null, mixed $value = null): static + public function orWhereNot(array|Closure|self|QueryBuilder|Relation|Expression|string $column, mixed $operator = null, mixed $value = null): static { return $this->whereNot($column, $operator, $value, 'or'); } @@ -383,7 +383,7 @@ public function orWhereNot(array|Closure|Expression|string $column, mixed $opera /** * Add an "order by" clause for a timestamp to the query. */ - public function latest(Expression|string|null $column = null): static + public function latest(Closure|self|QueryBuilder|Relation|Expression|string|null $column = null): static { if (is_null($column)) { $column = $this->model->getCreatedAtColumn() ?? 'created_at'; @@ -397,7 +397,7 @@ public function latest(Expression|string|null $column = null): static /** * Add an "order by" clause for a timestamp to the query. */ - public function oldest(Expression|string|null $column = null): static + public function oldest(Closure|self|QueryBuilder|Relation|Expression|string|null $column = null): static { if (is_null($column)) { $column = $this->model->getCreatedAtColumn() ?? 'created_at'; diff --git a/src/database/src/Eloquent/Concerns/QueriesRelationships.php b/src/database/src/Eloquent/Concerns/QueriesRelationships.php index 83c1bc57e..d130d224d 100644 --- a/src/database/src/Eloquent/Concerns/QueriesRelationships.php +++ b/src/database/src/Eloquent/Concerns/QueriesRelationships.php @@ -775,10 +775,6 @@ public function withAggregate(mixed $relations, Expression|string $column, ?stri return $this; } - if (is_null($this->query->columns)) { - $this->query->select([$this->query->from . '.*']); - } - $relations = is_array($relations) ? $relations : [$relations]; foreach ($this->parseWithRelations($relations) as $name => $constraints) { @@ -849,6 +845,12 @@ public function withAggregate(mixed $relations, Expression|string $column, ?stri ) ); + $this->assertNoTimeoutOnRelationshipConstraint($query); + + if (is_null($this->query->columns)) { + $this->query->select([$this->query->from . '.*']); + } + if ($function === 'exists') { $this->selectRaw( sprintf('exists(%s) as %s', $query->toSql(), $this->getQuery()->grammar->wrap($alias)), @@ -938,10 +940,13 @@ public function withExists(string|array $relation): static protected function addHasWhere(Builder $hasQuery, Relation $relation, string $operator, Expression|int $count, string $boolean): static { $hasQuery->mergeConstraintsFrom($relation->getQuery()); + $query = $hasQuery->toBase(); + + $this->assertNoTimeoutOnRelationshipConstraint($query); return $this->canUseExistsForExistenceCheck($operator, $count) - ? $this->addWhereExistsQuery($hasQuery->toBase(), $boolean, $operator === '<' && $count === 1) - : $this->addWhereCountQuery($hasQuery->toBase(), $operator, $count, $boolean); + ? $this->addWhereExistsQuery($query, $boolean, $operator === '<' && $count === 1) + : $this->addWhereCountQuery($query, $operator, $count, $boolean); } /** @@ -1002,6 +1007,20 @@ protected function addWhereCountQuery(QueryBuilder $query, string $operator = '> ); } + /** + * Ensure a relationship constraint does not carry a statement-level timeout. + * + * @throws InvalidArgumentException + */ + protected function assertNoTimeoutOnRelationshipConstraint(QueryBuilder $query): void + { + if ($query->timeout !== null) { + throw new InvalidArgumentException( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.' + ); + } + } + /** * Get the "has relation" base query instance. * diff --git a/tests/Database/DatabaseEloquentBuilderTest.php b/tests/Database/DatabaseEloquentBuilderTest.php index ddeabaada..0c7662c1c 100755 --- a/tests/Database/DatabaseEloquentBuilderTest.php +++ b/tests/Database/DatabaseEloquentBuilderTest.php @@ -1405,6 +1405,48 @@ public function testOrWhereNot() $this->assertEquals($builder, $result); } + public function testQueryableWhereForwardersAcceptBuilderAndRelationSubqueries(): void + { + $model = new ModelParentStub; + $model->foo_id = 7; + $connection = $this->mockConnectionForModel($model, 'SQLite'); + $subquery = $connection->query() + ->select('score') + ->from('scores') + ->where('active', true); + + $builder = $model->newQuery() + ->where($model->foo(), '>', 5) + ->orWhere($subquery, '<', 4) + ->whereNot($subquery, '=', 3) + ->orWhereNot($subquery, '=', 2); + + $this->assertSame( + 'select * from "model_parent_stubs" where (select * from "model_close_related_stubs" where "model_close_related_stubs"."id" = ?) > ? or (select "score" from "scores" where "active" = ?) < ? and not (select "score" from "scores" where "active" = ?) = ? or not (select "score" from "scores" where "active" = ?) = ?', + $builder->toSql() + ); + $this->assertSame([7, 5, true, 4, true, 3, true, 2], $builder->getBindings()); + } + + public function testFirstWhereAcceptsRelationSubquery(): void + { + $model = new ModelParentStub; + $model->foo_id = 7; + $connection = $this->mockConnectionForModel($model, 'SQLite'); + $connection->shouldReceive('getName')->andReturn('database'); + $connection->expects('select')->with( + 'select * from "model_parent_stubs" where (select * from "model_close_related_stubs" where "model_close_related_stubs"."id" = ?) > ? limit 1', + [7, 5], + true, + [], + )->andReturn([['id' => 11]]); + + $result = $model->newQuery()->firstWhere($model->foo(), '>', 5); + + $this->assertInstanceOf(ModelParentStub::class, $result); + $this->assertSame(11, $result->id); + } + public function testRealQueryHigherOrderOrWhereScopes() { $model = new HigherOrderWhereScopeStub; @@ -1759,6 +1801,52 @@ public function testWithExists() $this->assertSame('select "model_parent_stubs".*, exists(select * from "model_close_related_stubs" where "model_parent_stubs"."foo_id" = "model_close_related_stubs"."id") as "foo_exists" from "model_parent_stubs"', $builder->toSql()); } + public function testWithExistsRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void + { + $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $builder->withExists(['foo' => function ($query): void { + $query->where('active', true)->timeout(2); + }]); + + $this->fail('Expected the relationship constraint timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + + public function testWithCountRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void + { + $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $builder->withCount(['foo' => function ($query): void { + $query->where('active', true)->timeout(2); + }]); + + $this->fail('Expected the relationship constraint timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + public function testWithExistsAndSelect() { $model = new ModelParentStub; @@ -1913,6 +2001,52 @@ public function testHasWithConstraintsAndHavingInSubqueryWithCount() $this->assertEquals(['baz', 'qux', 'quuux'], $builder->getBindings()); } + public function testRelationshipExistsRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void + { + $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $builder->whereHas('foo', function ($query): void { + $query->where('active', true)->timeout(2); + }); + + $this->fail('Expected the relationship constraint timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + + public function testRelationshipCountRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void + { + $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $builder->whereHas('foo', function ($query): void { + $query->where('active', true)->timeout(2); + }, '>=', 2); + + $this->fail('Expected the relationship constraint timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + public function testWithCountAndConstraintsWithBindingInSelectSub() { $model = new ModelParentStub; @@ -2757,6 +2891,32 @@ public function testLatestWithColumn() $builder->latest('foo'); } + public function testLatestAndOldestAcceptQueryableSubqueries(): void + { + $model = new ModelParentStub; + $model->foo_id = 7; + $this->mockConnectionForModel($model, 'SQLite'); + + $latest = $model->newQuery()->latest($model->foo()); + + $this->assertSame( + 'select * from "model_parent_stubs" order by (select * from "model_close_related_stubs" where "model_close_related_stubs"."id" = ?) desc', + $latest->toSql() + ); + $this->assertSame([7], $latest->getBindings()); + + $subquery = $model->foo()->getRelated()->newQuery() + ->select('score') + ->where('active', true); + $oldest = $model->newQuery()->oldest($subquery); + + $this->assertSame( + 'select * from "model_parent_stubs" order by (select "score" from "model_close_related_stubs" where "active" = ?) asc', + $oldest->toSql() + ); + $this->assertSame([true], $oldest->getBindings()); + } + public function testOldestWithoutColumnWithCreatedAt() { $model = $this->getMockModel(); From c412a09145446c49772a0bed85efec95fec8f116 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:19:04 +0000 Subject: [PATCH 4/7] docs(database): define timeout and collision boundaries Document query timeouts as outer select-statement limits on MySQL and MariaDB, including the rejection of timed embedded queries and EXPLAIN statements. Explain the caller-owned transaction retry required when repeatable-read snapshots cannot observe a concurrent create-or-first winner. Rename the transaction retry guidance around the broader concurrency errors it actually detects while making clear that unique violations are not retried automatically. --- src/docs/database.md | 6 +++--- src/docs/eloquent.md | 3 +++ src/docs/queries.md | 9 ++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/docs/database.md b/src/docs/database.md index 9fc12c728..1c2c4aace 100644 --- a/src/docs/database.md +++ b/src/docs/database.md @@ -510,10 +510,10 @@ DB::transaction(function () { }); ``` - -#### Handling Deadlocks + +#### Handling Concurrency Errors -The `transaction` method accepts an optional second argument which defines the number of times a transaction should be retried when a deadlock occurs. Once these attempts have been exhausted, an exception will be thrown: +The `transaction` method accepts an optional second argument which defines the number of times a transaction should be attempted. After a complete rollback, Hypervel retries detected deadlocks, serialization failures, and database lock errors. Unique constraint violations are not retried. Once the configured attempts have been exhausted, the exception will be thrown: ```php use Hypervel\Support\Facades\DB; diff --git a/src/docs/eloquent.md b/src/docs/eloquent.md index 226713272..ab96a7775 100644 --- a/src/docs/eloquent.md +++ b/src/docs/eloquent.md @@ -760,6 +760,9 @@ $flight = Flight::firstOrNew( ); ``` +> [!NOTE] +> If `firstOrCreate` or `updateOrCreate` encounters a concurrent insert, it attempts to retrieve the winning row from the write connection. Inside a repeatable-read transaction, a row committed after the transaction's snapshot may remain invisible, in which case the original unique constraint violation is rethrown. For idempotent collision handling in this situation, retry the complete transaction from outside it. + ### Retrieving Aggregates diff --git a/src/docs/queries.md b/src/docs/queries.md index 55d4f461e..255fa94c6 100644 --- a/src/docs/queries.md +++ b/src/docs/queries.md @@ -324,14 +324,21 @@ $users = $query->addSelect('age')->get(); #### Query Timeouts -When using MariaDB or MySQL, the `timeout` method may be used to limit a select query's execution time in seconds: +When using MariaDB or MySQL, the `timeout` method may be used to limit a complete select statement's execution time in seconds. Apply the timeout to the outer query after composing any subqueries or unions: ```php +$activeMemberships = DB::table('memberships') + ->select('user_id') + ->where('active', true); + $users = DB::table('users') + ->whereIn('id', $activeMemberships) ->timeout(2) ->get(); ``` +Embedded queries cannot define independent timeouts, and a timed query cannot be passed to the `explain` method. The timeout applies only to select statements; it does not limit insert, update, or delete statements, nor does it provide a transaction-wide lock wait timeout. + #### Index Hints From 246f3291db2b683ad9b797b88104c8cc8cd708f1 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:19:15 +0000 Subject: [PATCH 5/7] docs(plans): record database correctness design Capture the final reviewed design for truthful Eloquent collision recovery, exact pivot membership proof, statement-owned MySQL and MariaDB timeouts, retained-fragment grammar ownership, and queryable Relation parity. Record the supported transaction boundaries, performance constraints, driver-specific verification matrix, regression coverage, and the rule that opaque cross-database expressions must qualify their own database-owned references. --- ...collision-and-query-timeout-correctness.md | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) create mode 100644 docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md diff --git a/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md b/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md new file mode 100644 index 000000000..a5880d6a2 --- /dev/null +++ b/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md @@ -0,0 +1,264 @@ +# Database Eloquent collision and query timeout correctness plan + +## Status and objective + +Implement one Components database correction that makes Eloquent create-or-retrieve collision fallbacks truthful and makes the documented MySQL/MariaDB query timeout apply to the complete executed select statement. + +The change keeps Laravel's public Eloquent and query-builder APIs. It adds no public framework surface, process-global state, worker-lifetime cache, connection mutation, retry loop, or extra work on ordinary Eloquent success paths. PostgreSQL and SQLite timeout behavior remains unchanged because the public timeout contract is explicitly MySQL/MariaDB-only. + +Preserve the documented MariaDB timeout rather than narrowing the feature to MySQL. MariaDB has a native single-statement primitive that implements the existing contract without session mutation or a new public API. + +This plan is the authoritative design for the Components work that must land before Workflow execution Work Order 3 resumes. + +## Verified defects and boundaries + +### Eloquent collision fallbacks + +The generic Eloquent builder and direct has-one/has-many fallback already read from the write PDO. Two relation variants do not: + +- `HasOneOrManyThrough::createOrFirst()` catches an insert collision and rereads through the relation without `useWritePdo()`. +- `BelongsToMany::createOrFirst()` catches a related-model insert collision and rereads the related table without `useWritePdo()`. + +Those reads can miss the winning row on a configured replica. Keep every existing builder shape and mutation rule; add write routing only at these two missing sites. Do not normalize the other fallbacks, add locks, or clone builders. The through relation already mutates itself with `where()`, while calls forwarded from the related model create a fresh Eloquent builder. + +Two `BelongsToMany` pivot-attach catches can also report success for the wrong unique violation: + +- `firstOrCreate()` swallows every pivot insert unique violation and returns the related model. +- `createOrFirst()` re-queries the joined relation by related attributes, which does not prove that the exact related model was attached and can select another matching model. + +After an attach collision, success is valid only when the exact intended relation membership is visible on the write PDO. `newPivotStatementForId()` already includes the parent key, related key, configured `wherePivot*` predicates, and `MorphToMany`'s morph discriminator. Reuse that query rather than adding relation-specific SQL or a new pivot API. + +PostgreSQL repeatable-read snapshots cannot see rows committed after the caller's snapshot. A locking read does not fix that and can break grouped, union, or one-of-many relation shapes on supported engines. Generic Eloquent cannot restart arbitrary caller work or identify which unique constraint was intended. When exact membership or the winning model is not visible, preserve and rethrow the original unique violation; callers that need idempotent behavior inside repeatable-read must retry their complete owning transaction. + +### Query timeout statement placement + +`Builder::timeout()` is documented for MySQL and MariaDB selects. The current implementation has five correctness gaps: + +- `MariaDbGrammar` inherits MySQL's `MAX_EXECUTION_TIME` optimizer hint. Supported MariaDB 10/11 releases ignore that hint; their portable per-statement form is `SET STATEMENT max_statement_time= FOR `. +- MySQL requires `MAX_EXECUTION_TIME` after the first `SELECT` and applies it to the complete statement. `exists()`, union aggregates, and grouped pagination currently decorate an inner select instead of the executed outer statement. +- The current MySQL replacement is anchored to `^select`, but an ordinary compiled union starts with `(select`; a timed union therefore receives no hint. +- A timed builder embedded as a subquery, relationship aggregate, relationship existence constraint, or union member carries a statement-level setting into a location where it cannot represent an independent timeout. +- `explain()` does not execute selected rows, while MariaDB's prefix cannot be nested after `EXPLAIN`. Keeping the timeout would either be ignored or produce invalid SQL. +- Retained where-subquery and exists builders are assembled by the outer grammar, so two same-database connections with different table prefixes can silently target the outer connection's table. Union members keep their own grammar but compile through its public decorating entry point. A child timed after composition can therefore emit statement decoration inside the outer statement even though the same child may legitimately be executed separately with its own timeout. + +Real MySQL and MariaDB checks confirmed: + +- MySQL's existing hint interrupts both a long select and a blocked `SELECT ... FOR UPDATE`. +- MariaDB 10.11 ignores that hint, while `SET STATEMENT max_statement_time=... FOR SELECT ...` interrupts both a long select and a blocked locking select. +- MySQL accepts its hint after the first `SELECT` inside a parenthesized union, and MariaDB accepts `SET STATEMENT ... FOR` before the complete parenthesized union statement. +- MariaDB reports its timeout in seconds; MySQL's hint remains in milliseconds. + +The API remains select-only. It supports top-level locking selects, but it is not a DML timeout or a complete transaction lock-wait mechanism. Workflow must not use it as a replacement for its engine-specific session lock-wait checks. + +### Queryable subquery type parity + +Laravel deliberately treats `Relation` as queryable because it forwards to an Eloquent builder, but the `@param` annotations on many Query and Eloquent Builder methods were never updated when that runtime support was added. Hypervel converted those stale annotations into native types, making supported subquery calls fail with `TypeError`. Several forwarding wrappers are also natively narrower than the methods they call, rejecting Query or Eloquent builders before the existing `isQueryable()` logic can handle them. + +One related runtime defect exists in both frameworks: `whereSub()` recognizes a Relation but stores it for `Grammar::compileSelect()`, which requires a Query Builder. Cross-database Relation subqueries also read `$relation->from`, although Relation has no property forwarding, and can corrupt the underlying table prefix. Normalize Relations at the existing builder boundaries rather than adding another query abstraction. + +The invariant is: no method may declare a parameter type narrower than the method it forwards to, and no parameter type may be narrower than what its own body accepts through `isQueryable()`. + +## Final implementation + +### Truthful Eloquent recovery + +1. In `HasOneOrManyThrough::createOrFirst()`, route only the collision fallback to the write PDO: + + ```php + return $this->useWritePdo()->where($attributes)->first() ?? throw $exception; + ``` + +2. In the first `BelongsToMany::createOrFirst()` fallback, keep the related model's fresh builder and route the read to the write PDO before `first()`. + +3. Add one protected `BelongsToMany::hasAttachedPivot(Model $instance): bool` predicate beside the create-or-retrieve methods: + + ```php + protected function hasAttachedPivot(Model $instance): bool + { + return $this->newPivotStatementForId($instance->getKey()) + ->useWritePdo() + ->exists(); + } + ``` + +4. Capture the pivot attach exception in both methods. Return the already-selected related model only when `hasAttachedPivot()` succeeds; otherwise rethrow that attach exception. Refactor `createOrFirst()`'s `tap()` expression into a direct local `$instance`, attach attempt, exact-membership check, and return so the collision subject cannot be lost or replaced. + +5. Do not lock the fallback query, classify every unique violation as a concurrency error, or add generic retries. The write-PDO correction fixes replica visibility where the current transaction can see the winner; the exact predicate prevents false success without claiming to solve caller-owned snapshot isolation. + +6. Type the create-or-retrieve, save, and create methods as returning plain related models. Those paths attach a pivot row but do not hydrate a `pivot` property; retain the pivot intersection only for models loaded through the relation query. + +### Statement-owned query timeout + +1. Split the base select grammar at a protected raw-assembly seam and make its public entry point own final-statement decoration: + + ```php + public function compileSelect(Builder $query): string + { + return $this->compileSelectTimeout( + $query, + $this->compileSelectQuery($query), + ); + } + + protected function compileSelectQuery(Builder $query): string + { + // Existing select assembly, including group-limit and union handling. + } + + protected function compileSelectTimeout(Builder $query, string $sql): string + { + return $sql; + } + ``` + + Keep the raw assembler and decorator protected. One base entry point owns the rule that only the complete executed statement is decorated; driver grammars only supply syntax. Do not add `compileSubSelect()`, `toSubSql()`, a public timeout capability API, or another grammar abstraction. + +2. Make `compileSelect()` and `compileExists()` the only complete-statement decorating entry points. Every embedded select fragment uses `compileSelectQuery()`: + + - `compileUnionAggregate()` uses the raw assembler for its derived table; the public `compileSelect()` applies the timeout to the aggregate statement. + - `compileExists()` uses the raw assembler for its inner query, builds `select exists(...)`, then calls `compileSelectTimeout()` on that outer statement. + - retained `whereSub`, `whereExists`, `whereNotExists`, and union members use their own grammar's raw assembler, so connection-owned details such as table prefixes remain correct while later child timeout changes affect standalone execution but cannot decorate the containing statement; + - SQLite and PostgreSQL update/delete rewrites use the raw assembler for their internal row-identifier selects. + + The fragment grammar owns raw fragment assembly; the outer grammar owns only the surrounding column/operator, exists/not-exists syntax, union conjunction, and union wrapping. PHP permits the shared declaring class to invoke the protected raw assembler on sibling driver grammar instances and still dispatches to the runtime override. Do not route fragments back through public `compileSelect()` or assemble them with the outer grammar. + + Delete `SQLiteGrammar::compileGroupLimit()`. Hypervel supports SQLite 3.26+, so its pre-3.25 fallback is unreachable; if reached it silently removes the per-group limit, and on supported versions it needlessly resolves PDO to inspect the server version during compilation. The base window-function compiler is the only valid supported path. + +3. Remove `MySqlGrammar::compileSelect()` and move its hint injection into `compileSelectTimeout()`, keeping the integer-seconds-to-milliseconds conversion. Match only the compiler-owned MySQL statement prefix, preserving any leading union parentheses and placing the hint after the first `SELECT`: + + ```php + return preg_replace( + '/^(\(*)select\b/i', + '${1}select /*+ MAX_EXECUTION_TIME(' . $milliseconds . ') */', + $sql, + 1, + ); + ``` + + Do not use an unanchored search or generic SQL parser. The raw assembler emits either `select...` or one or more wrapping parentheses followed by `select...`; the narrow prefix replacement fixes ordinary unions without touching nested selects. + +4. Override only `compileSelectTimeout()` in `MariaDbGrammar`: + + ```php + return $query->timeout === null + ? $sql + : 'SET STATEMENT max_statement_time=' . $query->timeout . ' FOR ' . $sql; + ``` + + The timeout is a validated positive integer, so it is emitted as a literal and adds no binding or session mutation. + +5. In grouped/having pagination counts, copy the timeout from the inner clone to the new outer count builder. Clear it from the clone before `toSql()` so generated SQL and bindings describe one timed executed statement. Simple pagination counts already execute their cloned aggregate directly and keep the timeout unchanged. + +6. Reject a timeout on a builder when it is accepted as a statement-producing part of another query. Add one protected Query Builder assertion and reuse it at the four distinct acceptance paths after closures and Eloquent builders are normalized to one scope-applied Query Builder snapshot: + + - `parseSub()` for select/from/join/order, queryable `whereIn()` / `whereNotIn()`, and other parsed subqueries, including `Relation` inputs; + - `whereSub()`; + - `addWhereExistsQuery()`; and + - `union()`. + + Inspect the exact Query Builder snapshot that will be retained. This catches timeouts applied by global scopes and prevents SQL and bindings from being produced by separate scope applications. Throw `InvalidArgumentException` with: + + > An embedded query cannot define its own timeout. Apply the timeout to the outer query instead. + + Assert before cross-database prefixing or mutating the accepted child, storing it, or merging its bindings. Callers may have performed earlier fluent mutations of the outer builder; throwing does not roll those back. The outer builder may still be timed and may contain any number of untimed subqueries or union members. + + Normalize accepted Eloquent union members to `toBase()` before the guard, storage, and binding harvest. This gives the retained union one scope-applied Query Builder snapshot for timeout inspection, SQL, and bindings and lets the grammar's raw compiler receive its declared type. Do not clone members or promise that later Eloquent scope changes alter an attached union. + +7. Reuse one protected local assertion in `QueriesRelationships`. Apply it to every `withAggregate()` arm before adding the default selection: the `exists` arm compiles with direct `toSql()`, while the other arms would otherwise reach Query Builder's generic guard only after that selection changed. Apply it once in `addHasWhere()` after constraint merging and before its exists/count branch so both paths use the relationship-specific diagnostic before storing the child or merging its bindings. Throw: + + > A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead. + + Keep this helper local; a shared trait, exception type, or public timeout accessor would add machinery without another consumer. + +8. In `ExplainsQueries::explain()`, reject a non-null timeout before calling `toSql()` with: + + > A query timeout cannot be applied to an EXPLAIN statement. Clear the timeout before calling explain(). + + Do not silently strip the timeout or generate invalid `EXPLAIN SET STATEMENT ...` SQL. + +9. Leave PostgreSQL and SQLite's documented timeout support unchanged. Untimed queries, update/delete rewrites, bindings, read/write routing, and connection/session state keep their current behavior. The base raw-assembly and no-op decorator calls add no branch, query, network round trip, allocation proportional to result size, or worker-lifetime state. + +### Queryable subquery type correction + +1. Normalize `Relation` at the start of `Query\Builder::parseSub()` with `getQuery()`, then normalize Eloquent Builder once with `toBase()`. Timeout inspection, cross-database prefixing, SQL, and bindings then use the same scope-applied Query Builder snapshot. Narrow `prependDatabaseNameIfCrossDatabaseQuery()` and `assertNoTimeoutOnEmbeddedQuery()` to Query Builder. Cross-database qualification applies only to a plain-string source; an absent source remains unchanged, while an opaque raw/derived-table expression must already qualify any database-owned references it contains. This fixes cross-database Relation table qualification, prevents repeated global-scope application, and catches timeouts applied by global scopes without another branch in downstream callers. +2. In `whereSub()`, normalize Query Builders directly and call `toBase()` on both Eloquent Builder and Relation inputs: + + ```php + $query = $callback instanceof self ? $callback : $callback->toBase(); + ``` + +3. Add `Relation` to the native and PHPDoc unions for every Query Builder surface whose body accepts queryable subqueries: `createSub()`, `selectSub()`, `from()` / `fromSub()`, every subquery join and lateral-join variant including `straightJoinSub()`, `whereSub()`, `whereBetween()` / `whereBetweenColumns()`, `orderBy()`, and `insertUsing()` / `insertOrIgnoreUsing()`. +4. Make Query Builder forwarding wrappers match their callees: `orWhere()`, `whereNot()`, `orWhereNot()`, all six Between/BetweenColumns wrappers, and `orderByDesc()`, `latest()`, `oldest()`, `reorder()`, and `reorderDesc()`. Correct the queryable element PHPDoc unions on `whereAll()` / `orWhereAll()`, `whereAny()` / `orWhereAny()`, and `whereNone()` / `orWhereNone()`. +5. Make Eloquent Builder's application-facing `where()`, `firstWhere()`, `orWhere()`, `whereNot()`, `orWhereNot()`, `latest()`, and `oldest()` accept the Query Builder, Eloquent Builder, and Relation values handled by the Query Builder methods they forward to. +6. Keep explicit unions rather than using the empty query-builder marker contract, which would accept implementations `parseSub()` cannot handle. Do not widen `whereExists*()` or `union*()`: those APIs do not use `isQueryable()` and intentionally require complete Query/Eloquent builders. Normalize the already-supported Eloquent union member as specified under statement-owned timeouts. Keep subquery `from()` aliases required and do not add exception-state rollback machinery to fluent builder methods. + +## Documentation + +- Update `src/docs/queries.md` to describe `timeout()` as an outer select-statement limit on MySQL/MariaDB. Show that it belongs on the outer builder, state that timed embedded builders and `explain()` are rejected, and do not present it as a DML or transaction-wide lock-wait control. +- Add one note beside `firstOrCreate()` / `updateOrCreate()` in `src/docs/eloquent.md`: under repeatable-read, a concurrent row committed after the caller's snapshot can remain invisible and the unique violation is rethrown; idempotent collision handling must retry the complete owning transaction from outside it. Relationship docs already lead to this section, so do not duplicate the note. +- Rename `src/docs/database.md`'s “Handling Deadlocks” section to “Handling Concurrency Errors”. Explain that `transaction(..., attempts:)` retries detected deadlocks, serialization failures, and database lock errors after a complete rollback, but does not retry unique violations. +- Keep low-level grammar and relation implementation details in source docblocks/comments. Add a short source comment only where final-statement timeout placement or exact-pivot proof is not clear from the code itself. + +## Testing plan + +### Eloquent unit and integration coverage + +- Update `DatabaseEloquentHasManyThroughCreateOrFirstTest` and `DatabaseEloquentBelongsToManyCreateOrFirstTest` so the two fallback selects are explicitly write-routed. Preserve all existing create, retrieve, update, closure-value, transaction, and model-state assertions. +- Cover both pivot attach catches with: + - exact parent/related membership visible on the write PDO returns the already-selected related model; + - an independent pivot unique constraint with no exact membership rethrows the same attach-violation instance; + - configured `wherePivot`, `wherePivotIn`, `wherePivotNull`, and `wherePivotBetween` predicates remain part of the proof; and + - `MorphToMany` includes the morph discriminator, so another morph type cannot prove membership. +- Add one SQLite read/write-split integration regression using separate temporary read and write databases. Seed the winning related/through row only on the writer, leave equivalent empty schemas on the reader, force the insert collision on the writer, and prove each corrected fallback returns the writer's row. Keep sticky reads disabled so the test cannot pass because a failed write changed connection state. +- Add a shared real-database pivot regression that runs on SQLite, MySQL, MariaDB, and PostgreSQL: a different pivot row occupies a second unique key, the attempted exact pivot is absent, and both create-or-retrieve methods rethrow instead of reporting attachment success. +- Keep repeatable-read behavior as a documented transaction-owner responsibility; do not build a timing-dependent test that pretends Eloquent can make a stale snapshot current. + +### Query grammar and builder coverage + +- Extend MySQL grammar tests for simple, distinct, aggregate, ordinary union, union aggregate, `exists()`, top-level locking select, timeout clearing, and outer placement with untimed subqueries. Preserve exact SQL and binding-order assertions, including the hint inside the first parenthesized union `SELECT`. +- Add the matching MariaDB grammar tests, asserting `SET STATEMENT max_statement_time= FOR` occurs once at the statement root for simple, aggregate, ordinary union, union aggregate, `exists()`, and locking selects. +- In `DatabaseQueryBuilderTest`, cover grouped/having pagination transferring the timeout to the outer count, an outer timeout with untimed parsed/where-exists/union members, and rejection at each of the four timed embedded-builder acceptance paths. Pin the exact diagnostic and prove the rejected child is not stored and its bindings are not merged; where a specific entry point performs no earlier outer mutation, also pin its unchanged outer state. +- Cover retained where-subquery, exists, and union children under both MySQL and MariaDB: after attaching an untimed child, setting its timeout must leave exactly one decoration at the outer statement root and none in the child fragment, while compiling that child standalone still applies its timeout. +- With stock grammars bound to differently prefixed same-database connections, assert that scalar where-subquery, exists, not-exists, and union fragments retain the child connection's prefix. +- Cover Eloquent parsed-subquery and union inputs whose global scopes set timeouts, proving each scope runs once and the scope-applied snapshot is rejected. Also cover an Eloquent union member with a binding-bearing global scope, pinning exact SQL and bindings through acceptance-time normalization. Do not encode later scope mutation of an attached union as supported behavior. +- Add exact base and offset group-limit SQL/binding assertions. Existing SQLite/PostgreSQL update/delete rewrite tests must remain byte-identical after their internal compiler calls move to the raw assembler. +- In `DatabaseEloquentBuilderTest`, cover `withExists()`, another `withAggregate()` arm, and both the default exists and count relationship paths, pinning the relationship-specific diagnostic before the child is embedded or its bindings are merged. +- Cover timed `explain()` rejection and unchanged untimed explanation SQL. +- Where the existing MySQL/MariaDB grammar test files are touched, add the missing `: void` return type to `testToRawSql()` under the repository's full-typing rule, and pin timeout clearing on both drivers. + +### Queryable type coverage + +- Use real model relations with mocked connections and assert exact SQL and binding order through representative public Query Builder paths: aliased select/from, cross-database table qualification, `whereSub()`, a subquery join including the straight-join wrapper, insert-using, Between/BetweenColumns wrappers, and order wrappers. +- Cover a cross-database subquery sourced through `fromSub()` with an already-qualified inner source, plus a tableless cross-database subquery, and prove both remain unchanged during qualification. +- Cover `orWhere()` / `whereNot()` with a plain Query Builder as well as Relation compilation so the Query, Eloquent, and Relation union arms cannot be accidentally omitted. +- Cover Eloquent Builder's public where/firstWhere, or/where-not, and latest/oldest families. Keep coverage family-level; do not add a reflection harness or one test per trivial forwarding alias. + +### Real timeout enforcement + +- Add driver-owned `QueryTimeoutTest` classes under the existing MySQL and MariaDB integration directories so the database workflow discovers them. +- For each engine, use a query that exceeds a one-second limit and assert the engine's timeout error, not only elapsed wall time. Cover ordinary execution with `SLEEP`, `exists()` with `SLEEP` in a predicate over a one-row probe table so the optimizer cannot discard it, and `unionAll()` with `SLEEP` in a returned arm so both compiler entry points and real union-wide enforcement are proven. +- Hold a row lock on a second connection and assert a timed `SELECT ... FOR UPDATE` is interrupted with the engine's timeout error. Use bounded coordination and exception-safe cleanup; never leave an open transaction or probe table. +- Run MariaDB coverage on the supported 10/11 matrix and MySQL coverage on the supported 8/9 matrix. This pins both the older MariaDB syntax requirement and MySQL's first-`SELECT` hint placement. + +## Implementation order and verification + +1. Correct the two Eloquent write routes and exact pivot predicate, updating and running each existing relation test file immediately. +2. Add and run the read/write-split and real-database pivot regressions before moving to timeout compilation. +3. Add the raw/final select grammar seam, MySQL/MariaDB decorators, internal wrapper placement, pagination transfer, and fail-fast guards. Update and run each grammar/builder test file as it is changed. +4. Correct the complete Query/Eloquent Builder queryable type surface and normalization rules, then run the changed builder test files immediately. +5. Add and run the real MySQL and MariaDB timeout tests through the driver workflow. +6. Update the three documentation sections with the public behavior and transaction boundary. +7. Run the complete database unit suite and all four real database integration groups with `bin/run-database-tests.sh`. +8. Run `composer fix` once at the completed implementation checkpoint. After fixes, run the affected targeted tests and repeat the full checkpoint only when the correction can affect another package or driver. +9. Freshly review every changed caller and callee for Laravel API compatibility, named/protected extension points, false success, query/binding order, read/write routing, transaction isolation, coroutine/worker state, hot-path query or allocation cost, duplicated logic, and overengineering. Then complete adversarial peer review before commit. + +## References + +- Current Laravel 12.x relation shapes: [BelongsToMany](https://github.com/laravel/framework/blob/12.x/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php), [HasOneOrManyThrough](https://github.com/laravel/framework/blob/12.x/src/Illuminate/Database/Eloquent/Relations/HasOneOrManyThrough.php). +- Current Laravel 12.x compiler shapes: [base query grammar](https://github.com/laravel/framework/blob/12.x/src/Illuminate/Database/Query/Grammars/Grammar.php), [MySQL grammar](https://github.com/laravel/framework/blob/12.x/src/Illuminate/Database/Query/Grammars/MySqlGrammar.php). +- Laravel's [Relation subquery support](https://github.com/laravel/framework/pull/33180), whose runtime queryable surface is broader than its retained parameter annotations. +- MySQL `MAX_EXECUTION_TIME` statement rules for [8.0](https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-execution-time) and [9.1](https://dev.mysql.com/doc/refman/9.1/en/optimizer-hints.html#optimizer-hints-execution-time). +- [MariaDB `SET STATEMENT`](https://mariadb.com/docs/server/reference/sql-statements/administrative-sql-statements/set-commands/set-statement) and [statement timeout behavior](https://mariadb.com/docs/server/ha-and-performance/optimization-and-tuning/query-optimizations/aborting-statements). +- [PostgreSQL transaction isolation and complete-transaction retry](https://www.postgresql.org/docs/current/transaction-iso.html). + +The work is complete only when collision fallbacks never report unproven relation membership, every fallback that must observe a recent winner reads from the writer, timeout SQL appears exactly once on the executed outer statement, timed embedded snapshots fail before they are stored or their bindings are merged, retained children cannot later emit nested decoration, and the full supported database matrix and repository checks pass. From dbec7f65f9d11168402dae575e189bd5cd72d525 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:27:54 +0000 Subject: [PATCH 6/7] test(types): align pivot assertions with relation returns Update BelongsToMany type fixtures for create, save, attach, and create-or-retrieve paths that return plain related models without hydrating a pivot property. Keep pivot intersections on models loaded through the relation and narrow mixed paths to their truthful common Role type. This matches the corrected production PHPDoc and prevents static analysis from approving unsafe pivot access. --- types/Database/Eloquent/Relations.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/types/Database/Eloquent/Relations.php b/types/Database/Eloquent/Relations.php index 90ce2abbb..01cb98722 100644 --- a/types/Database/Eloquent/Relations.php +++ b/types/Database/Eloquent/Relations.php @@ -52,7 +52,7 @@ function test(User $user, Post $post, Comment $comment, ChildUser $child): void assertType('Hypervel\Database\Eloquent\Collection', $user->roles()->findOrFail([1])); assertType('42|Hypervel\Database\Eloquent\Collection', $user->roles()->findOr([1], fn () => 42)); assertType('42|Hypervel\Database\Eloquent\Collection', $user->roles()->findOr([1], callback: fn () => 42)); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->findOrNew(1)); + assertType('Hypervel\Types\Relations\Role', $user->roles()->findOrNew(1)); assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->findOrFail(1)); assertType('(Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot})|null', $user->roles()->find(1)); assertType('42|(Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot})', $user->roles()->findOr(1, fn () => 42)); @@ -61,20 +61,20 @@ function test(User $user, Post $post, Comment $comment, ChildUser $child): void assertType('42|(Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot})', $user->roles()->firstOr(fn () => 42)); assertType('42|(Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot})', $user->roles()->firstOr(callback: fn () => 42)); assertType('(Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot})|null', $user->roles()->firstWhere('foo')); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->firstOrNew()); + assertType('Hypervel\Types\Relations\Role', $user->roles()->firstOrNew()); assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->firstOrFail()); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->firstOrCreate()); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->create()); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->createOrFirst()); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->updateOrCreate([])); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->save(new Role)); - assertType('Hypervel\Types\Relations\Role&object{pivot: Hypervel\Database\Eloquent\Relations\Pivot}', $user->roles()->saveQuietly(new Role)); + assertType('Hypervel\Types\Relations\Role', $user->roles()->firstOrCreate()); + assertType('Hypervel\Types\Relations\Role', $user->roles()->create()); + assertType('Hypervel\Types\Relations\Role', $user->roles()->createOrFirst()); + assertType('Hypervel\Types\Relations\Role', $user->roles()->updateOrCreate([])); + assertType('Hypervel\Types\Relations\Role', $user->roles()->save(new Role)); + assertType('Hypervel\Types\Relations\Role', $user->roles()->saveQuietly(new Role)); $roles = $user->roles()->getResults(); assertType('iterable<(int|string), Hypervel\Types\Relations\Role>', $user->roles()->saveMany($roles)); assertType('iterable<(int|string), Hypervel\Types\Relations\Role>', $user->roles()->saveMany($roles->all())); assertType('iterable<(int|string), Hypervel\Types\Relations\Role>', $user->roles()->saveManyQuietly($roles)); assertType('iterable<(int|string), Hypervel\Types\Relations\Role>', $user->roles()->saveManyQuietly($roles->all())); - assertType('array', $user->roles()->createMany($roles)); + assertType('array', $user->roles()->createMany($roles)); assertType('array{attached: array, detached: array, updated: array}', $user->roles()->sync($roles)); assertType('array{attached: array, detached: array, updated: array}', $user->roles()->syncWithoutDetaching($roles)); assertType('array{attached: array, detached: array, updated: array}', $user->roles()->syncWithPivotValues($roles, [])); From da857c03cfd112283c0ee6d9828765048c0ff418 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:12:23 +0000 Subject: [PATCH 7/7] Refine relationship timeout regression coverage Consolidate the four relationship timeout rejection tests behind one focused helper while preserving each public API path and the unchanged outer-query assertions.\n\nReplace the plan's implementation-only Relation exclusion rationale with the behavioral reason: an instance-bound relation would produce an uncorrelated exists subquery, while whereHas provides the correlated relationship API. Keep union inputs aligned with Laravel's declared builder contract. --- ...collision-and-query-timeout-correctness.md | 2 +- .../Database/DatabaseEloquentBuilderTest.php | 95 ++++++------------- 2 files changed, 32 insertions(+), 65 deletions(-) diff --git a/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md b/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md index a5880d6a2..40e9d092a 100644 --- a/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md +++ b/docs/plans/2026-08-12-1351-database-eloquent-collision-and-query-timeout-correctness.md @@ -190,7 +190,7 @@ The invariant is: no method may declare a parameter type narrower than the metho 3. Add `Relation` to the native and PHPDoc unions for every Query Builder surface whose body accepts queryable subqueries: `createSub()`, `selectSub()`, `from()` / `fromSub()`, every subquery join and lateral-join variant including `straightJoinSub()`, `whereSub()`, `whereBetween()` / `whereBetweenColumns()`, `orderBy()`, and `insertUsing()` / `insertOrIgnoreUsing()`. 4. Make Query Builder forwarding wrappers match their callees: `orWhere()`, `whereNot()`, `orWhereNot()`, all six Between/BetweenColumns wrappers, and `orderByDesc()`, `latest()`, `oldest()`, `reorder()`, and `reorderDesc()`. Correct the queryable element PHPDoc unions on `whereAll()` / `orWhereAll()`, `whereAny()` / `orWhereAny()`, and `whereNone()` / `orWhereNone()`. 5. Make Eloquent Builder's application-facing `where()`, `firstWhere()`, `orWhere()`, `whereNot()`, `orWhereNot()`, `latest()`, and `oldest()` accept the Query Builder, Eloquent Builder, and Relation values handled by the Query Builder methods they forward to. -6. Keep explicit unions rather than using the empty query-builder marker contract, which would accept implementations `parseSub()` cannot handle. Do not widen `whereExists*()` or `union*()`: those APIs do not use `isQueryable()` and intentionally require complete Query/Eloquent builders. Normalize the already-supported Eloquent union member as specified under statement-owned timeouts. Keep subquery `from()` aliases required and do not add exception-state rollback machinery to fluent builder methods. +6. Keep explicit unions rather than using the empty query-builder marker contract, which would accept implementations `parseSub()` cannot handle. Do not widen `whereExists*()` to `Relation`: a relation instance is already constrained to one parent, so embedding it as an exists subquery would be uncorrelated with the outer rows and could silently match every row or none; callers need Eloquent's correlated `whereHas*()` APIs. Do not widen `union*()` either: Laravel's declared contract requires complete Query/Eloquent builders, and Relation support would add an unneeded API. Normalize the already-supported Eloquent union member as specified under statement-owned timeouts. Keep subquery `from()` aliases required and do not add exception-state rollback machinery to fluent builder methods. ## Documentation diff --git a/tests/Database/DatabaseEloquentBuilderTest.php b/tests/Database/DatabaseEloquentBuilderTest.php index 0c7662c1c..a7a17a3fd 100755 --- a/tests/Database/DatabaseEloquentBuilderTest.php +++ b/tests/Database/DatabaseEloquentBuilderTest.php @@ -1803,48 +1803,20 @@ public function testWithExists() public function testWithExistsRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void { - $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); - $sql = $builder->toSql(); - $bindings = $builder->getBindings(); - - try { + $this->assertRelationshipConstraintTimeoutRejected(function (Builder $builder): void { $builder->withExists(['foo' => function ($query): void { $query->where('active', true)->timeout(2); }]); - - $this->fail('Expected the relationship constraint timeout to be rejected.'); - } catch (InvalidArgumentException $exception) { - $this->assertSame( - 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', - $exception->getMessage() - ); - } - - $this->assertSame($sql, $builder->toSql()); - $this->assertSame($bindings, $builder->getBindings()); + }); } public function testWithCountRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void { - $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); - $sql = $builder->toSql(); - $bindings = $builder->getBindings(); - - try { + $this->assertRelationshipConstraintTimeoutRejected(function (Builder $builder): void { $builder->withCount(['foo' => function ($query): void { $query->where('active', true)->timeout(2); }]); - - $this->fail('Expected the relationship constraint timeout to be rejected.'); - } catch (InvalidArgumentException $exception) { - $this->assertSame( - 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', - $exception->getMessage() - ); - } - - $this->assertSame($sql, $builder->toSql()); - $this->assertSame($bindings, $builder->getBindings()); + }); } public function testWithExistsAndSelect() @@ -2003,48 +1975,20 @@ public function testHasWithConstraintsAndHavingInSubqueryWithCount() public function testRelationshipExistsRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void { - $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); - $sql = $builder->toSql(); - $bindings = $builder->getBindings(); - - try { + $this->assertRelationshipConstraintTimeoutRejected(function (Builder $builder): void { $builder->whereHas('foo', function ($query): void { $query->where('active', true)->timeout(2); }); - - $this->fail('Expected the relationship constraint timeout to be rejected.'); - } catch (InvalidArgumentException $exception) { - $this->assertSame( - 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', - $exception->getMessage() - ); - } - - $this->assertSame($sql, $builder->toSql()); - $this->assertSame($bindings, $builder->getBindings()); + }); } public function testRelationshipCountRejectsConstraintTimeoutBeforeEmbeddingTheConstraint(): void { - $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); - $sql = $builder->toSql(); - $bindings = $builder->getBindings(); - - try { + $this->assertRelationshipConstraintTimeoutRejected(function (Builder $builder): void { $builder->whereHas('foo', function ($query): void { $query->where('active', true)->timeout(2); }, '>=', 2); - - $this->fail('Expected the relationship constraint timeout to be rejected.'); - } catch (InvalidArgumentException $exception) { - $this->assertSame( - 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', - $exception->getMessage() - ); - } - - $this->assertSame($sql, $builder->toSql()); - $this->assertSame($bindings, $builder->getBindings()); + }); } public function testWithCountAndConstraintsWithBindingInSelectSub() @@ -3347,6 +3291,29 @@ public function testIncrementEachWithoutTimestamps(): void $this->assertSame(1, $result); } + /** + * Assert that a relationship constraint timeout is rejected before it is embedded. + */ + protected function assertRelationshipConstraintTimeoutRejected(Closure $accept): void + { + $builder = (new ModelParentStub)->newQuery()->where('tenant_id', 7); + $sql = $builder->toSql(); + $bindings = $builder->getBindings(); + + try { + $accept($builder); + $this->fail('Expected the relationship constraint timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'A relationship constraint cannot define its own query timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $builder->toSql()); + $this->assertSame($bindings, $builder->getBindings()); + } + protected function mockConnectionForModel($model, $database) { $grammarClass = 'Hypervel\Database\Query\Grammars\\' . $database . 'Grammar';