fix(database): fix multiple model properties joining the same table#2080
Open
laylatichy wants to merge 14 commits intotempestphp:3.xfrom
Open
fix(database): fix multiple model properties joining the same table#2080laylatichy wants to merge 14 commits intotempestphp:3.xfrom
laylatichy wants to merge 14 commits intotempestphp:3.xfrom
Conversation
Benchmark ResultsComparison of Open to see the benchmark resultsNo benchmark changes above ±5%. Generated by phpbench against commit ff5fde7 |
67e340c to
f1de44f
Compare
brendt
requested changes
Mar 31, 2026
When a model has multiple BelongsTo relations to the same table, the query builder generated duplicate JOINs with the same bare table name. Fix by using the property name as the SQL alias for root-level eager relations (parent='') in HasTableAlias::getTableAlias(). Add getOwnerTableAlias() so nested relations reference their parent's aliased name instead of the raw table name. Applied consistently across BelongsTo, HasOne, HasMany, and BelongsToMany relation types. Fixes tempestphp#2079
Add test models and assertions for multiple BelongsTo relations to the same table: distinct JOIN aliases, full table.column syntax, and SELECT field aliasing. Update BelongsToMany parent join expectation to use aliased owner reference.
…ading Verify actual data fetching works for: - two BelongsTo to same table via explicit .with() - two #[Eager] BelongsTo to same table - parent -> child with two eager -> subchild chain Update SelectQueryBuilder test expectations to match aliased SQL.
The target table is now aliased by property name when loaded via eager/with relations. fix(database): use named arg in str() call
…ion types Move rewriteTablePrefix from BelongsTo to HasTableAlias trait so all relation types can rewrite explicit table.column references when the table is aliased. Applied in HasMany, HasOne, BelongsToMany, HasOneThrough, and HasManyThrough.
Wrap aliases in backticks from getTableAlias/getOwnerTableAlias when they differ from the raw table name. JoinStatement::compile() converts backticks to double quotes for PostgreSQL and strips them for SQLite. This prevents reserved keywords like 'user' from causing syntax errors when used as property names that become table aliases.
…longsToMany Verify distinct aliased JOINs when two properties of the same relation type point to the same table. Covers root-level and nested (parent → child with 2 same-table → subchild) scenarios.
…ence Rename for clarity and apply consistently across all relation types. Strip table prefix from explicit table.column join params and re-prefix with the correct alias, preserving cross-table references when the specified table differs from the expected one.
Replace separate BelongsTo-only and SQL-string tests with a single MultipleSameTableRelationsTest covering all relation types with actual data: BelongsTo, HasMany, and nested parent → child with two same-table → subchild chains.
brendt
approved these changes
Mar 31, 2026
Member
brendt
left a comment
There was a problem hiding this comment.
Can you fix the merge conflicts? After that I'm fine to merge
FieldStatement now uses double quotes for PostgreSQL field identifiers instead of backticks. JoinStatement converts backticks to double quotes for PostgreSQL and strips them for SQLite. This ensures table aliases that are reserved keywords (like 'user') work across all dialects. Shorten test table names to avoid MySQL FK constraint name length limit.
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.
f1de44f to
5b6da79
Compare
Verify the query builder correctly flags duplicate target tables: - single relation: not aliased - duplicate target tables: both aliased - mixed relations: only duplicates aliased - nested relations under non-duplicate: not aliased
5b6da79 to
d6ac901
Compare
Wrap aliases in backticks from getTableAlias when withPropertyNameAlias is set. JoinStatement::compile() converts backticks to double quotes for PostgreSQL and strips them for SQLite. FieldStatement already handles this via DatabaseDialect::quoteIdentifier(). This ensures aliased table names are consistently quoted across SELECT fields and JOIN clauses, preventing PostgreSQL case-sensitivity mismatches.
Contributor
Author
|
@brendt resolved |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #2079
tldr we need to alias using property name instead of table name if duplicate relations are present