From ec1db7c43de54a5f033db489abde6172c3d1e2ed Mon Sep 17 00:00:00 2001 From: RenHai Wen <70972819+people-sea@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:24:44 +0800 Subject: [PATCH] fix: respect table prefixes in query builder relationship aggregates (#20386) * fix: respect table prefixes in query builder relationship aggregates * fix: preserve one-of-many relationship semantics in query builder aggregates * fix: respect relationship method constraints in query builder aggregates Aggregate subqueries were built from a fresh query on the related model, so constraints defined inside the relationship method itself were never applied. An aggregate over a relationship such as `hasMany(Post::class)->where('is_published', true)` silently aggregated every related row, and `wherePivot()` on a `BelongsToMany` was ignored in the same way. Merge the relationship's own query into the subquery with `mergeConstraintsFrom()`, matching how Laravel builds its own `withAggregate()` subqueries. --------- Co-authored-by: Dan Harrin --- .../Concerns/CanAggregateRelationships.php | 46 +++---- ...ate_prefixed_query_builder_items_table.php | 22 ++++ .../Livewire/UsersQueryBuilderTable.php | 4 + .../src/Fixtures/Models/QueryBuilderItem.php | 18 +++ tests/src/Fixtures/Models/User.php | 15 +++ tests/src/Tables/Filters/QueryBuilderTest.php | 114 ++++++++++++++++++ 6 files changed, 189 insertions(+), 30 deletions(-) create mode 100644 tests/database/migrations/0026_create_prefixed_query_builder_items_table.php create mode 100644 tests/src/Fixtures/Models/QueryBuilderItem.php diff --git a/packages/query-builder/src/Constraints/NumberConstraint/Operators/Concerns/CanAggregateRelationships.php b/packages/query-builder/src/Constraints/NumberConstraint/Operators/Concerns/CanAggregateRelationships.php index 1c858ccb9c..7861548509 100644 --- a/packages/query-builder/src/Constraints/NumberConstraint/Operators/Concerns/CanAggregateRelationships.php +++ b/packages/query-builder/src/Constraints/NumberConstraint/Operators/Concerns/CanAggregateRelationships.php @@ -65,47 +65,33 @@ trait CanAggregateRelationships $modifyRelationshipQueryUsing = $this->getConstraint()->getModifyRelationshipQueryUsing(); /** @var Relation $relationship */ - $relationship = $query->getModel()->{$relationshipName}(); + $relationship = Relation::noConstraints( + static fn (): Relation => $query->getModel()->{$relationshipName}(), + ); $relatedModel = $relationship->getModel(); - $attributeForQuery = $relatedModel->qualifyColumn($attributeForQuery); $castType = $this->getNumericCastType($query); - if ($relationship instanceof BelongsToMany) { - $pivotTable = $relationship->getTable(); - $foreignPivotKey = $relationship->getQualifiedForeignPivotKeyName(); - $relatedPivotKey = $relationship->getQualifiedRelatedPivotKeyName(); - $parentKey = $relationship->getQualifiedParentKeyName(); - $relatedKey = $relationship->getQualifiedRelatedKeyName(); - - $subQuery = $relatedModel->query() - ->selectRaw("cast({$aggregate}({$attributeForQuery}) as {$castType})") - ->join($pivotTable, $relatedKey, '=', $relatedPivotKey) - ->whereColumn($foreignPivotKey, $parentKey); - - if ($modifyRelationshipQueryUsing) { - $subQuery = $this->evaluate($modifyRelationshipQueryUsing, ['query' => $subQuery]) ?? $subQuery; - } - - return $query->whereRaw("({$subQuery->toSql()}) {$operator} ?", [...$subQuery->getBindings(), $value]); + if (! ($relationship instanceof BelongsToMany) && ! ($relationship instanceof HasOneOrMany)) { + throw new LogicException('Relationship type [' . get_class($relationship) . '] is not supported for aggregate queries.'); } - if ($relationship instanceof HasOneOrMany) { - $foreignKeyName = $relationship->getQualifiedForeignKeyName(); - $parentKeyName = $relationship->getQualifiedParentKeyName(); + $subQuery = $relationship->getRelationExistenceQuery( + $relatedModel->newQueryWithoutRelationships(), + $query, + [], + )->mergeConstraintsFrom($relationship->getQuery()); - $subQuery = $relatedModel->query() - ->selectRaw("cast({$aggregate}({$attributeForQuery}) as {$castType})") - ->whereColumn($foreignKeyName, $parentKeyName); + $attributeForQuery = $subQuery->qualifyColumn($attributeForQuery); + $attributeForQuery = $subQuery->getQuery()->getGrammar()->wrap($attributeForQuery); - if ($modifyRelationshipQueryUsing) { - $subQuery = $this->evaluate($modifyRelationshipQueryUsing, ['query' => $subQuery]) ?? $subQuery; - } + $subQuery->selectRaw("cast({$aggregate}({$attributeForQuery}) as {$castType})"); - return $query->whereRaw("({$subQuery->toSql()}) {$operator} ?", [...$subQuery->getBindings(), $value]); + if ($modifyRelationshipQueryUsing) { + $subQuery = $this->evaluate($modifyRelationshipQueryUsing, ['query' => $subQuery]) ?? $subQuery; } - throw new LogicException('Relationship type [' . get_class($relationship) . '] is not supported for aggregate queries.'); + return $query->whereRaw("({$subQuery->toSql()}) {$operator} ?", [...$subQuery->getBindings(), $value]); } protected function getAggregateSelect(): Select diff --git a/tests/database/migrations/0026_create_prefixed_query_builder_items_table.php b/tests/database/migrations/0026_create_prefixed_query_builder_items_table.php new file mode 100644 index 0000000000..7fee1e2013 --- /dev/null +++ b/tests/database/migrations/0026_create_prefixed_query_builder_items_table.php @@ -0,0 +1,22 @@ +id(); + $table->foreignId('parent_id')->nullable(); + $table->unsignedInteger('length'); + }); + } + + public function down(): void + { + Schema::dropIfExists('fo_query_builder_items'); + } +}; diff --git a/tests/src/Fixtures/Livewire/UsersQueryBuilderTable.php b/tests/src/Fixtures/Livewire/UsersQueryBuilderTable.php index 0bc3f961ca..7bf015e7ec 100644 --- a/tests/src/Fixtures/Livewire/UsersQueryBuilderTable.php +++ b/tests/src/Fixtures/Livewire/UsersQueryBuilderTable.php @@ -43,6 +43,10 @@ class UsersQueryBuilderTable extends Component implements HasActions, HasSchemas ->label('Posts Rating Aggregate (Dot Syntax)'), NumberConstraint::make('teams.budget') ->label('Teams Budget Aggregate (Dot Syntax)'), + NumberConstraint::make('publishedPosts.rating') + ->label('Published Posts Rating Aggregate'), + NumberConstraint::make('ownedTeams.budget') + ->label('Owned Teams Budget Aggregate'), RelationshipConstraint::make('posts') ->multiple() ->selectable( diff --git a/tests/src/Fixtures/Models/QueryBuilderItem.php b/tests/src/Fixtures/Models/QueryBuilderItem.php new file mode 100644 index 0000000000..43fa2e6058 --- /dev/null +++ b/tests/src/Fixtures/Models/QueryBuilderItem.php @@ -0,0 +1,18 @@ +hasMany(self::class, 'parent_id'); + } +} diff --git a/tests/src/Fixtures/Models/User.php b/tests/src/Fixtures/Models/User.php index 6e6487ef98..293bc464db 100644 --- a/tests/src/Fixtures/Models/User.php +++ b/tests/src/Fixtures/Models/User.php @@ -68,6 +68,16 @@ class User extends Authenticatable implements FilamentUser, HasAppAuthentication return $this->hasOne(Post::class, 'author_id')->where('is_published', true); } + public function latestPost(): HasOne + { + return $this->hasOne(Post::class, 'author_id')->latestOfMany(); + } + + public function publishedPosts(): HasMany + { + return $this->hasMany(Post::class, 'author_id')->where('is_published', true); + } + protected static function newFactory() { return UserFactory::new(); @@ -93,6 +103,11 @@ class User extends Authenticatable implements FilamentUser, HasAppAuthentication return $this->belongsToMany(Team::class); } + public function ownedTeams(): BelongsToMany + { + return $this->belongsToMany(Team::class)->wherePivot('role', 'owner'); + } + public function profile(): HasOne { return $this->hasOne(Profile::class); diff --git a/tests/src/Tables/Filters/QueryBuilderTest.php b/tests/src/Tables/Filters/QueryBuilderTest.php index 8616e7ed7d..d70f43a2e0 100644 --- a/tests/src/Tables/Filters/QueryBuilderTest.php +++ b/tests/src/Tables/Filters/QueryBuilderTest.php @@ -27,6 +27,7 @@ use Filament\Tests\Fixtures\Livewire\UsersQueryBuilderTable; use Filament\Tests\Fixtures\Livewire\UsersQueryBuilderTableWithScopedPostsCount; use Filament\Tests\Fixtures\Livewire\UsersQueryBuilderTableWithScopedPostsRatingAggregate; use Filament\Tests\Fixtures\Models\Post; +use Filament\Tests\Fixtures\Models\QueryBuilderItem; use Filament\Tests\Fixtures\Models\Team; use Filament\Tests\Fixtures\Models\User; use Filament\Tests\Tables\TestCase; @@ -1726,6 +1727,119 @@ describe('relationship method constraints', function (): void { ->assertCanNotSeeTableRecords([$lowMinUser]); }); + it('can filter self-related records using number constraint aggregate with a connection table prefix', function (): void { + $databaseConnection = QueryBuilderItem::query()->getConnection(); + $originalTablePrefix = $databaseConnection->getTablePrefix(); + + $databaseConnection->setTablePrefix('fo_'); + + try { + $matchingParent = QueryBuilderItem::query()->create(['length' => 1]); + $matchingParent->children()->create(['length' => 700]); + + $nonMatchingParent = QueryBuilderItem::query()->create(['length' => 1]); + $nonMatchingParent->children()->create(['length' => 699]); + + $constraint = NumberConstraint::make('children.length'); + + $operator = IsMinOperator::make() + ->constraint($constraint) + ->settings(['number' => 700, 'aggregate' => 'min']); + + $filteredQuery = $operator->applyToBaseQuery(QueryBuilderItem::query()); + + expect($filteredQuery->pluck('id')->all()) + ->toBe([$matchingParent->id]); + } finally { + $databaseConnection->setTablePrefix($originalTablePrefix); + } + }); + + it('can filter records using number constraint aggregate on a `latestOfMany()` relationship', function (): void { + $matchingUser = User::factory()->create(); + Post::factory()->create(['author_id' => $matchingUser->id, 'rating' => 3]); + Post::factory()->create(['author_id' => $matchingUser->id, 'rating' => 8]); + + $nonMatchingUser = User::factory()->create(); + Post::factory()->create(['author_id' => $nonMatchingUser->id, 'rating' => 8]); + Post::factory()->create(['author_id' => $nonMatchingUser->id, 'rating' => 3]); + + $constraint = NumberConstraint::make('latestPost.rating'); + + $operator = IsMinOperator::make() + ->constraint($constraint) + ->settings(['number' => 7, 'aggregate' => 'min']); + + $filteredQuery = $operator->applyToBaseQuery(User::query()); + + expect($filteredQuery->pluck('id')->all()) + ->toBe([$matchingUser->id]); + }); + + it('applies constraints defined in the relationship method to the aggregate subquery', function (): void { + $matchingUser = User::factory()->create(); + Post::factory()->create([ + 'author_id' => $matchingUser->id, + 'is_published' => true, + 'rating' => 9, + ]); + Post::factory()->create([ + 'author_id' => $matchingUser->id, + 'is_published' => false, + 'rating' => 1, + ]); + + $nonMatchingUser = User::factory()->create(); + Post::factory()->create([ + 'author_id' => $nonMatchingUser->id, + 'is_published' => true, + 'rating' => 1, + ]); + Post::factory()->create([ + 'author_id' => $nonMatchingUser->id, + 'is_published' => false, + 'rating' => 9, + ]); + + livewire(UsersQueryBuilderTable::class) + ->assertCanSeeTableRecords([$matchingUser, $nonMatchingUser]) + ->tap(applyQueryBuilderFilter([ + [ + 'type' => 'publishedPosts.rating', + 'data' => [ + 'operator' => 'isMin', + 'settings' => ['number' => 7, 'aggregate' => 'min'], + ], + ], + ])) + ->assertCanSeeTableRecords([$matchingUser]) + ->assertCanNotSeeTableRecords([$nonMatchingUser]); + }); + + it('applies `wherePivot()` constraints defined in the relationship method to the aggregate subquery', function (): void { + $matchingUser = User::factory()->create(); + $matchingUser->teams()->attach(Team::factory()->create(['budget' => 5000])->id, ['role' => 'owner']); + $matchingUser->teams()->attach(Team::factory()->create(['budget' => 100])->id, ['role' => 'member']); + + $nonMatchingUser = User::factory()->create(); + $nonMatchingUser->teams()->attach(Team::factory()->create(['budget' => 100])->id, ['role' => 'owner']); + $nonMatchingUser->teams()->attach(Team::factory()->create(['budget' => 5000])->id, ['role' => 'member']); + + livewire(UsersQueryBuilderTable::class) + ->assertCanSeeTableRecords([$matchingUser, $nonMatchingUser]) + ->tap(applyQueryBuilderFilter([ + [ + 'type' => 'ownedTeams.budget', + 'data' => [ + 'operator' => 'isMin', + 'settings' => ['number' => 1000, 'aggregate' => 'min'], + ], + ], + ])) + ->assertCanSeeTableRecords([$matchingUser]) + ->assertCanNotSeeTableRecords([$nonMatchingUser]); + }); + it('can filter records using number constraint with max aggregate on relationship', function (): void { // Create user with at least one very high rating $highMaxUser = User::factory()->create();