fix: replace having alias with whereRaw for postgresql compatibility#125
Conversation
PostgreSQL does not allow referencing column aliases in HAVING clauses. Use whereRaw with the full subquery expression instead.
There was a problem hiding this comment.
Pull request overview
This PR updates the TopTeamsTableWidget query to be PostgreSQL-compatible by avoiding the use of a HAVING clause that references a SELECT alias (records_count), instead reusing the full computed expression in the filter.
Changes:
- Refactors the computed
records_countSQL into a reusable expression string. - Replaces
having('records_count', '>', 0)withwhereRaw("({$expression}) > 0")to avoid alias usage inHAVING.
| private function buildRecordsCountExpression(string $systemSource, string $startStr, string $endStr): string | ||
| { | ||
| $subqueries = collect(self::ENTITY_TABLES)->map( | ||
| fn (string $table): string => "(SELECT COUNT(*) FROM {$table} WHERE {$table}.team_id = teams.id AND {$table}.deleted_at IS NULL AND {$table}.creation_source != '{$systemSource}' AND {$table}.created_at BETWEEN '{$startStr}' AND '{$endStr}')" | ||
| ); |
There was a problem hiding this comment.
buildRecordsCountExpression() interpolates $systemSource, $startStr, and $endStr directly into raw SQL strings that are then used in both DB::raw() and whereRaw(). This is brittle (quoting/escaping) and bypasses query bindings; consider switching to selectRaw() / whereRaw() with ? placeholders + bindings (or building these subqueries with the query builder) so values are safely parameterized and won’t break if they contain quotes.
| return Team::query() | ||
| ->where('personal_team', false) | ||
| ->addSelect([ | ||
| 'teams.*', | ||
| $this->buildRecordsCountSelect($systemSource, $startStr, $endStr), | ||
| DB::raw("({$recordsCountExpression}) as records_count"), | ||
| DB::raw('(SELECT COUNT(*) FROM team_user WHERE team_user.team_id = teams.id) as members_count'), | ||
| DB::raw('(SELECT COUNT(*) FROM custom_fields WHERE custom_fields.tenant_id = teams.id) as custom_fields_count'), | ||
| $this->buildLastActivitySelect($systemSource), | ||
| ]) | ||
| ->having('records_count', '>', 0); | ||
| ->whereRaw("({$recordsCountExpression}) > 0"); |
There was a problem hiding this comment.
Filtering was changed from having('records_count', '>', 0) to whereRaw(...) for PostgreSQL compatibility; this is important query behavior and currently has no direct automated coverage. Please add a Pest test that executes the widget’s query (or renders the widget/table) and asserts teams with zero records in the period are excluded, so future refactors don’t regress PostgreSQL compatibility.
| $this->buildLastActivitySelect($systemSource), | ||
| ]) | ||
| ->having('records_count', '>', 0); | ||
| ->whereRaw("({$recordsCountExpression}) > 0"); |
There was a problem hiding this comment.
recordsCountExpression is now inlined in both the SELECT (to expose records_count) and the WHERE (to filter > 0). That can force the database to evaluate the same correlated subqueries twice per team. To avoid the double work, consider computing records_count in a subquery/CTE (or fromSub() wrapper) and then applying where('records_count', '>', 0) on the outer query.
| ->whereRaw("({$recordsCountExpression}) > 0"); | |
| ->having('records_count', '>', 0); |
Use query parameter placeholders instead of string interpolation for values in raw SQL expressions to prevent potential injection.
PostgreSQL does not allow referencing column aliases in HAVING clauses. Use whereRaw with the full subquery expression instead.