Skip to content

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

Open
laylatichy wants to merge 13 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 13 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 (7334926688b074d0f93e75b701c511dffc8106dd).

Open to see the benchmark results
Benchmark Set Mem. Peak Time Variability
ViewRenderBench(benchPlainHtml) - 21.956mb 0.00% 480.352μs -6.59% ±2.07% +54.72%
ViewRenderBench(benchExpressions) - 24.513mb 0.00% 509.175μs -7.17% ±2.55% +50.26%

Generated by phpbench against commit f1de44f

@laylatichy laylatichy force-pushed the fix-invalid-query-on-same-table-joins branch 2 times, most recently from 7dd250a to d999686 Compare March 27, 2026 11:21
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.
@laylatichy laylatichy force-pushed the fix-invalid-query-on-same-table-joins branch 4 times, most recently from 2fe3f2b to 67e340c Compare March 27, 2026 13:58
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.
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.
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 67e340c to f1de44f Compare March 27, 2026 14:12
@laylatichy laylatichy marked this pull request as ready for review March 27, 2026 14:26
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

1 participant