Skip to content

fix(database): fix multiple model properties joining the same table#2080

Open
laylatichy wants to merge 14 commits intotempestphp:3.xfrom
laylatichy:fix-invalid-query-on-same-table-joins
Open

fix(database): fix multiple model properties joining the same table#2080
laylatichy wants to merge 14 commits intotempestphp:3.xfrom
laylatichy:fix-invalid-query-on-same-table-joins

Conversation

@laylatichy
Copy link
Copy Markdown
Contributor

@laylatichy laylatichy commented Mar 27, 2026

fixes #2079

tldr we need to alias using property name instead of table name if duplicate relations are present

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 27, 2026

Benchmark Results

Comparison of fix-invalid-query-on-same-table-joins against 3.x (ccd398fe8a3ae55ec16808b96df38d63c2ccd82d).

Open to see the benchmark results

No benchmark changes above ±5%.

Generated by phpbench against commit ff5fde7

@laylatichy laylatichy force-pushed the fix-invalid-query-on-same-table-joins branch 7 times, most recently from 67e340c to f1de44f Compare March 27, 2026 14:12
@laylatichy laylatichy marked this pull request as ready for review March 27, 2026 14:26
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.
Copy link
Copy Markdown
Member

@brendt brendt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@laylatichy laylatichy force-pushed the fix-invalid-query-on-same-table-joins branch from f1de44f to 5b6da79 Compare March 31, 2026 11:28
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
@laylatichy laylatichy force-pushed the fix-invalid-query-on-same-table-joins branch from 5b6da79 to d6ac901 Compare March 31, 2026 11:38
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.
@laylatichy
Copy link
Copy Markdown
Contributor Author

@brendt resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple #[Eager] #[BelongsTo] relations to same table produce duplicate JOINs with wrong aliases

2 participants