Skip to content

fix(query-builder): nest child joins only when outer join semantics require it - #12793

Open
lazerg wants to merge 3 commits into
typeorm:masterfrom
lazerg:fix/12792-flat-nested-left-joins
Open

fix(query-builder): nest child joins only when outer join semantics require it#12793
lazerg wants to merge 3 commits into
typeorm:masterfrom
lazerg:fix/12792-flat-nested-left-joins

Conversation

@lazerg

@lazerg lazerg commented Aug 19, 2026

Copy link
Copy Markdown

Fixes #12792

Description of change

Since 1.1.0 a join gets wrapped in parentheses together with its children whenever it has any, so leftJoinAndSelect("u.profile", "p").leftJoinAndSelect("p.country", "c") now emits LEFT JOIN ("profile" "p" LEFT JOIN "country" "c" ON ...) ON ... instead of two flat LEFT JOINs. The nesting was added in #11137 to keep an inner child join from filtering out rows that its outer parent preserves, but it is applied to every combination, including left under left, where flat emission returns exactly the same rows. The parentheses pin the join order, and the reporter saw a seven-table query regress to ~19s averages on PostgreSQL 18 after upgrading.

So the parentheses are now gated on the case they were introduced for: an inner child join under a left joined parent. Every other combination goes back to the flat form 1.0.0 emitted. The behaviour tests from #11137 pass unchanged, and the two SQL string assertions that were updated in that PR for a left-under-left chain move back to the flat expectation.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • This pull request links a relevant issue using a closing keyword:
    Fixes #NNNN, Closes #NNNN, or Resolves #NNNN
  • There are new or updated tests validating the change (tests/**.test.ts)
  • Documentation has been updated to reflect this change (docs/docs/**.md) — N/A (no documented behaviour changes, only the emitted SQL shape)

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. #12792 test reference missing ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new regression tests are correctly located in the functional suite but do not identify issue
#12792 in a test comment. This omits the issue reference required for applicable issue-fix tests.
Code

test/functional/query-builder/join/query-builder-joins.test.ts[R399-400]

+        it("should not nest a child left join inside its left joined parent", () =>
+            Promise.all(
Evidence
PR Compliance ID 3 requires applicable issue-fix tests in test/functional to include an issue
reference in a test comment. The newly added regression test begins at line 399 without any comment
referencing #12792, and the repository contains no TypeScript reference to that issue.

Rule 3: Prefer functional tests over per-issue tests
test/functional/query-builder/join/query-builder-joins.test.ts[399-408]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The regression tests for issue `#12792` do not include the applicable issue reference required by the functional-test compliance rule.
## Issue Context
Add a concise comment identifying these cases as regression coverage for GitHub issue `#12792`, while keeping the tests in the functional suite.
## Fix Focus Areas
- test/functional/query-builder/join/query-builder-joins.test.ts[399-412]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


  • Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 2a4ba62 ⚖️ Balanced

Results up to commit N/A


🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. #12792 test reference missing 📘 Rule violation ⚙ Maintainability
Description
The new regression tests are correctly located in the functional suite but do not identify issue
#12792 in a test comment. This omits the issue reference required for applicable issue-fix tests.
Code

test/functional/query-builder/join/query-builder-joins.test.ts[R399-400]

+        it("should not nest a child left join inside its left joined parent", () =>
+            Promise.all(
Evidence
PR Compliance ID 3 requires applicable issue-fix tests in test/functional to include an issue
reference in a test comment. The newly added regression test begins at line 399 without any comment
referencing #12792, and the repository contains no TypeScript reference to that issue.

Rule 3: Prefer functional tests over per-issue tests
test/functional/query-builder/join/query-builder-joins.test.ts[399-408]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The regression tests for issue `#12792` do not include the applicable issue reference required by the functional-test compliance rule.
## Issue Context
Add a concise comment identifying these cases as regression coverage for GitHub issue `#12792`, while keeping the tests in the functional suite.
## Fix Focus Areas
- test/functional/query-builder/join/query-builder-joins.test.ts[399-412]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

Qodo Logo

@pkg-pr-new

pkg-pr-new Bot commented Aug 22, 2026

Copy link
Copy Markdown

commit: 2a4ba62

@Cprakhar
Cprakhar requested review from alumni and smith-xyz August 22, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linked-issue PR references an issue

Development

Successfully merging this pull request may close these issues.

Postgres: unconditional nested join parentheses in 1.1.0 constrain the planner and regress query plans

1 participant