refactor(database): only alias when duplicate target tables detected

laylatichy · gitbutler-client · commit 26dab63f7d3d · 2026-03-31T12:25:17.000+01:00
Instead of aliasing all root-level relations (which broke whereRaw with
bare table names), detect duplicate target tables in the query builder
and only alias those via withPropertyNameAlias(). Single-relation cases
keep bare table names, preserving backward compatibility.

Remove getOwnerTableAlias (now a no-op), revert FieldStatement/
JoinStatement/doc changes that were only needed for universal aliasing.
diff --git a/docs/1-essentials/03-database.md b/docs/1-essentials/03-database.md
@@ -272,7 +272,7 @@ The `through` parameter specifies the intermediate model class. The target model
 
 ```sql
 LEFT JOIN profiles ON profiles.author_id = authors.id
-LEFT JOIN addresses AS address ON address.profile_id = profiles.id
+LEFT JOIN addresses ON addresses.profile_id = profiles.id
 ```
 
 When conventions don't match, optional parameters can override the join fields:
diff --git a/packages/database/src/BelongsTo.php b/packages/database/src/BelongsTo.php
@@ -105,7 +105,7 @@ public function getJoinStatement(): JoinStatement
             ? sprintf('%s AS %s', $tableName, $tableAlias)
             : $tableName;
 
-        // LEFT JOIN authors AS author ON author.id = books.author_id
+        // LEFT JOIN authors ON authors.id = books.author_id
         return new JoinStatement(sprintf(
             'LEFT JOIN %s ON %s = %s',
             $tableRef,
@@ -171,7 +171,7 @@ private function isSelfReferencing(): bool
     private function getOwnerJoin(ModelInspector $ownerModel): string
     {
         $ownerJoin = $this->ownerJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());
+        $ownerTable = $ownerModel->getTableName();
 
         if ($ownerJoin && ! strpos($ownerJoin, '.')) {
             $ownerJoin = sprintf('%s.%s', $ownerTable, $ownerJoin);
diff --git a/packages/database/src/BelongsToMany.php b/packages/database/src/BelongsToMany.php
@@ -247,7 +247,7 @@ private function resolveOwnerJoin(
     private function resolveRelationJoin(ModelInspector $ownerModel): string
     {
         $relationJoin = $this->relationJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());
+        $ownerTable = $ownerModel->getTableName();
 
         if (
             $relationJoin
diff --git a/packages/database/src/Builder/QueryBuilders/SelectQueryBuilder.php b/packages/database/src/Builder/QueryBuilders/SelectQueryBuilder.php
@@ -500,6 +500,27 @@ public function getResolvedRelations(): array
             $relations = [...$relations, ...$resolvedRelations];
         }
 
+        return $this->shouldUsePropertyNameAlias($relations);
+    }
+
+    /**
+     * @param Relation[] $relations
+     *
+     * @return Relation[]
+     */
+    private function shouldUsePropertyNameAlias(array $relations): array
+    {
+        $rootTables = arr(input: $relations)
+            ->filter(filter: fn (Relation $_, string $path) => ! str_contains(haystack: $path, needle: '.'))
+            ->map(map: fn (Relation $relation) => inspect(model: $relation)->getTableName())
+            ->toArray();
+
+        $counts = array_count_values(array: $rootTables);
+
+        arr(input: $rootTables)
+            ->filter(filter: fn (string $table) => $counts[$table] > 1)
+            ->each(each: fn (string $_, string $path) => $relations[$path]->withPropertyNameAlias());
+
         return $relations;
     }
 
diff --git a/packages/database/src/HasMany.php b/packages/database/src/HasMany.php
@@ -189,7 +189,7 @@ private function isSelfReferencing(): bool
     private function getRelationJoin(ModelInspector $relationModel): string
     {
         $relationJoin = $this->relationJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $relationModel->getTableName());
+        $ownerTable = $relationModel->getTableName();
 
         if ($relationJoin && ! strpos($relationJoin, '.')) {
             $relationJoin = sprintf(
diff --git a/packages/database/src/HasManyThrough.php b/packages/database/src/HasManyThrough.php
@@ -216,7 +216,7 @@ private function resolveOwnerJoin(
     private function resolveRelationJoin(ModelInspector $ownerModel): string
     {
         $relationJoin = $this->relationJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());
+        $ownerTable = $ownerModel->getTableName();
 
         if (
             $relationJoin
diff --git a/packages/database/src/HasOne.php b/packages/database/src/HasOne.php
@@ -159,7 +159,7 @@ private function isSelfReferencing(): bool
     private function getRelationJoin(ModelInspector $relationModel): string
     {
         $relationJoin = $this->relationJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $relationModel->getTableName());
+        $ownerTable = $relationModel->getTableName();
 
         if ($relationJoin && ! strpos($relationJoin, '.')) {
             $relationJoin = sprintf(
diff --git a/packages/database/src/HasOneThrough.php b/packages/database/src/HasOneThrough.php
@@ -173,7 +173,7 @@ private function resolveOwnerJoin(
     private function resolveRelationJoin(ModelInspector $ownerModel): string
     {
         $relationJoin = $this->relationJoin;
-        $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());
+        $ownerTable = $ownerModel->getTableName();
 
         if (
             $relationJoin
diff --git a/packages/database/src/HasTableAlias.php b/packages/database/src/HasTableAlias.php
@@ -8,18 +8,25 @@
 
 trait HasTableAlias
 {
+    public bool $withPropertyNameAlias = false;
+
+    public function withPropertyNameAlias(): self
+    {
+        $this->withPropertyNameAlias = true;
+
+        return $this;
+    }
+
     private function getTableAlias(string $tableName): string
     {
         if ($this->parent === null) {
             return $tableName;
         }
 
         if ($this->parent === '') {
-            $alias = $this->property->getName();
-
-            return $alias === $tableName
-                ? $tableName
-                : str(string: $alias)->wrap('`')->toString();
+            return $this->withPropertyNameAlias
+                ? $this->property->getName()
+                : $tableName;
         }
 
         return str(string: $this->parent)
@@ -31,22 +38,6 @@ private function getTableAlias(string $tableName): string
                 '_',
                 $this->property->getName(),
             )
-            ->wrap('`')
-            ->toString();
-    }
-
-    private function getOwnerTableAlias(string $ownerTableName): string
-    {
-        if ($this->parent === null || $this->parent === '') {
-            return $ownerTableName;
-        }
-
-        return str(string: $this->parent)
-            ->replace(
-                search: '.',
-                replace: '_',
-            )
-            ->wrap('`')
             ->toString();
     }
 
diff --git a/packages/database/src/QueryStatements/FieldStatement.php b/packages/database/src/QueryStatements/FieldStatement.php
@@ -45,11 +45,11 @@ function (string $part) use ($dialect) {
                         return $part;
                     }
 
-                    return match ($dialect) {
-                        DatabaseDialect::SQLITE => $part,
-                        DatabaseDialect::POSTGRESQL => sprintf('"%s"', $part),
-                        default => sprintf('`%s`', $part),
-                    };
+                    if ($dialect === DatabaseDialect::SQLITE) {
+                        return $part;
+                    }
+
+                    return sprintf('`%s`', $part);
                 },
             )
             ->implode('.');
diff --git a/packages/database/src/QueryStatements/JoinStatement.php b/packages/database/src/QueryStatements/JoinStatement.php
@@ -18,13 +18,9 @@ public function compile(DatabaseDialect $dialect): string
         $statement = $this->statement;
 
         if (! str($statement)->lower()->startsWith(['join', 'inner join', 'left join', 'right join', 'full join', 'full outer join', 'self join'])) {
-            $statement = sprintf('INNER JOIN %s', $statement);
+            return sprintf('INNER JOIN %s', $statement);
         }
 
-        return match ($dialect) {
-            DatabaseDialect::POSTGRESQL => str_replace('`', '"', $statement),
-            DatabaseDialect::SQLITE => str_replace('`', '', $statement),
-            default => $statement,
-        };
+        return $statement;
     }
 }
diff --git a/packages/database/src/Relation.php b/packages/database/src/Relation.php
@@ -11,8 +11,12 @@ interface Relation extends PropertyAttribute
 {
     public string $name { get; }
 
+    public bool $withPropertyNameAlias { get; }
+
     public function setParent(string $name): self;
 
+    public function withPropertyNameAlias(): self;
+
     public function getSelectFields(): ImmutableArray;
 
     public function getJoinStatement(): JoinStatement;
diff --git a/packages/database/tests/QueryStatements/FieldStatementTest.php b/packages/database/tests/QueryStatements/FieldStatementTest.php
@@ -47,12 +47,12 @@ public function test_mysql(): void
     public function test_postgres(): void
     {
         $this->assertSame(
-            '"table"."field"',
+            '`table`.`field`',
             new FieldStatement('`table`.`field`')->compile(DatabaseDialect::POSTGRESQL),
         );
 
         $this->assertSame(
-            '"table"."field"',
+            '`table`.`field`',
             new FieldStatement('table.field')->compile(DatabaseDialect::POSTGRESQL),
         );
     }
diff --git a/tests/Integration/Database/Builder/SelectQueryBuilderTest.php b/tests/Integration/Database/Builder/SelectQueryBuilderTest.php
@@ -458,7 +458,7 @@ public function test_with_belongs_to_relation(): void
             ->build();
 
         $this->assertSameWithoutBackticks(
-            'SELECT books.id AS `books.id`, books.title AS `books.title`, books.author_id AS `books.author_id`, author.id AS `author.id`, author.name AS `author.name`, author.type AS `author.type`, author.publisher_id AS `author.publisher_id`, chapters.id AS `chapters.id`, chapters.title AS `chapters.title`, chapters.contents AS `chapters.contents`, chapters.book_id AS `chapters.book_id`, isbn.id AS `isbn.id`, isbn.value AS `isbn.value`, isbn.book_id AS `isbn.book_id` FROM `books` LEFT JOIN authors AS author ON author.id = books.author_id LEFT JOIN chapters ON chapters.book_id = books.id LEFT JOIN isbns AS isbn ON isbn.book_id = books.id',
+            'SELECT books.id AS `books.id`, books.title AS `books.title`, books.author_id AS `books.author_id`, authors.id AS `author.id`, authors.name AS `author.name`, authors.type AS `author.type`, authors.publisher_id AS `author.publisher_id`, chapters.id AS `chapters.id`, chapters.title AS `chapters.title`, chapters.contents AS `chapters.contents`, chapters.book_id AS `chapters.book_id`, isbns.id AS `isbn.id`, isbns.value AS `isbn.value`, isbns.book_id AS `isbn.book_id` FROM `books` LEFT JOIN authors ON authors.id = books.author_id LEFT JOIN chapters ON chapters.book_id = books.id LEFT JOIN isbns ON isbns.book_id = books.id',
             $query->compile(),
         );
     }
@@ -610,7 +610,7 @@ public function test_paginate_preserves_relations(): void
         $page1 = query(Chapter::class)
             ->select()
             ->with('book')
-            ->whereRaw('book.title = ?', 'LOTR 1')
+            ->whereRaw('books.title = ?', 'LOTR 1')
             ->paginate(itemsPerPage: 5, currentPage: 1);
 
         $this->assertSame(3, $page1->totalItems);
@@ -734,7 +734,7 @@ public function test_select_with_has_one_through_relation(): void
             ->build();
 
         $this->assertSameWithoutBackticks(
-            'SELECT tags.id AS `tags.id`, tags.label AS `tags.label`, topReviewer.id AS `topReviewer.id`, topReviewer.name AS `topReviewer.name`, topReviewer.book_review_id AS `topReviewer.book_review_id` FROM `tags` LEFT JOIN book_reviews ON book_reviews.tag_id = tags.id LEFT JOIN reviewers AS topReviewer ON topReviewer.book_review_id = book_reviews.id',
+            'SELECT tags.id AS `tags.id`, tags.label AS `tags.label`, reviewers.id AS `topReviewer.id`, reviewers.name AS `topReviewer.name`, reviewers.book_review_id AS `topReviewer.book_review_id` FROM `tags` LEFT JOIN book_reviews ON book_reviews.tag_id = tags.id LEFT JOIN reviewers ON reviewers.book_review_id = book_reviews.id',
             $query->compile(),
         );
     }
@@ -747,7 +747,7 @@ public function test_select_with_duplicate_belongs_to_many_target_table(): void
             ->build();
 
         $this->assertSameWithoutBackticks(
-            'SELECT users.id AS `users.id`, users.name AS `users.name`, users.role_id AS `users.role_id`, role.id AS `role.id`, role.name AS `role.name`, role_permissions.id AS `role.permissions.id`, role_permissions.label AS `role.permissions.label`, permissions.id AS `permissions.id`, permissions.label AS `permissions.label` FROM `users` LEFT JOIN roles AS role ON role.id = users.role_id LEFT JOIN permissions_roles ON permissions_roles.role_id = role.id LEFT JOIN permissions AS role_permissions ON role_permissions.id = permissions_roles.permission_id LEFT JOIN permissions_users ON permissions_users.user_id = users.id LEFT JOIN permissions ON permissions.id = permissions_users.permission_id',
+            'SELECT users.id AS `users.id`, users.name AS `users.name`, users.role_id AS `users.role_id`, roles.id AS `role.id`, roles.name AS `role.name`, role_permissions.id AS `role.permissions.id`, role_permissions.label AS `role.permissions.label`, permissions.id AS `permissions.id`, permissions.label AS `permissions.label` FROM `users` LEFT JOIN roles ON roles.id = users.role_id LEFT JOIN permissions_roles ON permissions_roles.role_id = roles.id LEFT JOIN permissions AS role_permissions ON role_permissions.id = permissions_roles.permission_id LEFT JOIN permissions_users ON permissions_users.user_id = users.id LEFT JOIN permissions ON permissions.id = permissions_users.permission_id',
             $query->compile(),
         );
     }
diff --git a/tests/Integration/Database/ModelInspector/BelongsToManyTest.php b/tests/Integration/Database/ModelInspector/BelongsToManyTest.php
@@ -67,7 +67,7 @@ public function test_belongs_to_many_with_parent_join_uses_alias(): void
             ->setParent(name: 'parent');
 
         $this->assertSame(
-            expected: 'LEFT JOIN owner_target ON owner_target.owner_id = parent.id LEFT JOIN target AS parent_targets ON parent_targets.id = owner_target.target_id',
+            expected: 'LEFT JOIN owner_target ON owner_target.owner_id = owner.id LEFT JOIN target AS parent_targets ON parent_targets.id = owner_target.target_id',
             actual: $relation
                 ->getJoinStatement()
                 ->compile(dialect: DatabaseDialect::SQLITE),
diff --git a/tests/Integration/Database/ModelInspector/BelongsToTest.php b/tests/Integration/Database/ModelInspector/BelongsToTest.php
@@ -107,8 +107,8 @@ public function test_multiple_belongs_to_same_table_generates_distinct_joins():
     {
         $model = inspect(BelongsToTestRoleWithMultipleSameTableRelationsModel::class);
 
-        $createdByRelation = $model->getRelation('createdBy')->setParent('');
-        $updatedByRelation = $model->getRelation('updatedBy')->setParent('');
+        $createdByRelation = $model->getRelation('createdBy')->setParent('')->withPropertyNameAlias();
+        $updatedByRelation = $model->getRelation('updatedBy')->setParent('')->withPropertyNameAlias();
 
         $this->assertEquals(
             'LEFT JOIN users AS createdBy ON createdBy.id = roles.created_by',
@@ -125,8 +125,8 @@ public function test_multiple_belongs_to_same_table_with_full_table_column_synta
     {
         $model = inspect(BelongsToTestRoleWithFullSpecRelationsModel::class);
 
-        $createdByRelation = $model->getRelation('created_by')->setParent('');
-        $updatedByRelation = $model->getRelation('updated_by')->setParent('');
+        $createdByRelation = $model->getRelation('created_by')->setParent('')->withPropertyNameAlias();
+        $updatedByRelation = $model->getRelation('updated_by')->setParent('')->withPropertyNameAlias();
 
         $this->assertEquals(
             'LEFT JOIN users AS created_by ON created_by.id = roles.created_by',
@@ -143,8 +143,8 @@ public function test_multiple_belongs_to_same_table_select_fields(): void
     {
         $model = inspect(BelongsToTestRoleWithMultipleSameTableRelationsModel::class);
 
-        $createdByFields = $model->getRelation('createdBy')->setParent('')->getSelectFields();
-        $updatedByFields = $model->getRelation('updatedBy')->setParent('')->getSelectFields();
+        $createdByFields = $model->getRelation('createdBy')->setParent('')->withPropertyNameAlias()->getSelectFields();
+        $updatedByFields = $model->getRelation('updatedBy')->setParent('')->withPropertyNameAlias()->getSelectFields();
 
         $this->assertSame(
             'createdBy.id AS `createdBy.id`',

Original file line number	Diff line number	Diff line change
`@@ -247,7 +247,7 @@ private function resolveOwnerJoin(`
`247`	`247`	`private function resolveRelationJoin(ModelInspector $ownerModel): string`
`248`	`248`	`{`
`249`	`249`	`$relationJoin = $this->relationJoin;`
`250`		`- $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());`
	`250`	`+ $ownerTable = $ownerModel->getTableName();`
`251`	`251`
`252`	`252`	`if (`
`253`	`253`	`$relationJoin`
Original file line number	Diff line number	Diff line change
`@@ -189,7 +189,7 @@ private function isSelfReferencing(): bool`
`189`	`189`	`private function getRelationJoin(ModelInspector $relationModel): string`
`190`	`190`	`{`
`191`	`191`	`$relationJoin = $this->relationJoin;`
`192`		`- $ownerTable = $this->getOwnerTableAlias(ownerTableName: $relationModel->getTableName());`
	`192`	`+ $ownerTable = $relationModel->getTableName();`
`193`	`193`
`194`	`194`	`if ($relationJoin && ! strpos($relationJoin, '.')) {`
`195`	`195`	`$relationJoin = sprintf(`
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ private function resolveOwnerJoin(`
`216`	`216`	`private function resolveRelationJoin(ModelInspector $ownerModel): string`
`217`	`217`	`{`
`218`	`218`	`$relationJoin = $this->relationJoin;`
`219`		`- $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());`
	`219`	`+ $ownerTable = $ownerModel->getTableName();`
`220`	`220`
`221`	`221`	`if (`
`222`	`222`	`$relationJoin`
Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ private function isSelfReferencing(): bool`
`159`	`159`	`private function getRelationJoin(ModelInspector $relationModel): string`
`160`	`160`	`{`
`161`	`161`	`$relationJoin = $this->relationJoin;`
`162`		`- $ownerTable = $this->getOwnerTableAlias(ownerTableName: $relationModel->getTableName());`
	`162`	`+ $ownerTable = $relationModel->getTableName();`
`163`	`163`
`164`	`164`	`if ($relationJoin && ! strpos($relationJoin, '.')) {`
`165`	`165`	`$relationJoin = sprintf(`
Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ private function resolveOwnerJoin(`
`173`	`173`	`private function resolveRelationJoin(ModelInspector $ownerModel): string`
`174`	`174`	`{`
`175`	`175`	`$relationJoin = $this->relationJoin;`
`176`		`- $ownerTable = $this->getOwnerTableAlias(ownerTableName: $ownerModel->getTableName());`
	`176`	`+ $ownerTable = $ownerModel->getTableName();`
`177`	`177`
`178`	`178`	`if (`
`179`	`179`	`$relationJoin`