fix(model): emit flat joins for belongsTo-chain nested includes (#3245) - #3249
Conversation
The $fromClause join builder decided whether to wrap inner joins in a parenthesized group bound to the LEFT OUTER JOIN purely from a regex on the include string (any `Intermediate(Target)` shape). That issue #449 HABTM/through grouping over-fired on plain belongsTo-chain nested includes such as `include="SecondaryContact(User)"`, nesting an inner join whose ON clause references the root FROM table — which scopes the root out and makes MySQL reject it with "Unknown column ... in 'on clause'". This was a regression from Wheels 2's flat joins. Consult the association metadata instead: only set needsNesting when the parenthesized intermediate association is a genuine hasMany/hasOne bridge (the OUTER-joined case the grouping was designed for). A belongsTo intermediate now falls through to the flat-join branch, restoring Wheels 2 behavior, while real HABTM/through includes still nest unchanged. Verified on SQLite and MySQL 9.7: crudSpec 163/0/0 (incl. new #3245 + #449 regression specs) and hasManyShortcutSpec 13/0/0; full model suite 923/0/0 on SQLite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <petera@pai.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR fixes the #3245 regression where findAll(include="Intermediate(Target)") with a belongsTo intermediate emitted a nested parenthesized join that scoped the root FROM table out of the inner join's ON clause (MySQL Unknown column ... in 'on clause'). The fix replaces the string-only Intermediate(Target) heuristic with an association-metadata lookup, nesting only when the parenthesized intermediate is a genuine hasMany/hasOne bridge (the #449 case). The change is tightly scoped, correct, and well-tested in both directions. Verdict: approve.
Correctness
The root cause and fix are accurate. vendor/wheels/model/sql.cfc:122-131 now derives hasThroughAssociation from the association metadata instead of the include string. Verified against the codebase:
.typeis set tobelongsTo/hasMany/hasOneinassociations.cfc:25/76/129and stored verbatim atassociations.cfc:213, so thevariables.wheels.class.associations[name].typelookup is valid.pos[2]/len[2]is capture group 1 (text before the open paren);ListLastofposts,usercorrectly yieldsuser. FormemberTeams(member)it yieldsmemberTeams.- The
StructKeyExistsguard means an unrecognized intermediate falls through to the flat-join branch — the safe Wheels 2 behavior. - The anchored regex single-paren/leading-position limitation is pre-existing and unchanged by this PR.
Cross-engine
Clean. Only StructKeyExists, ListFindNoCase, ListLast, Mid, Find, ReFindNoCase — no closures, reserved scopes, obj.map(), attributeCollection=arguments, or bare cfabort. The for loops are not inside a finally (invariant #12 N/A), and the Mid length comes from a one-or-more capture so it is always >= 1 (no Left(str,0)/zero-length hazard).
Tests
Both regression directions are covered in crudSpec.cfc (extends wheels.WheelsTest):
crudSpec.cfc:1271asserts flat joins for abelongsTo-chain include (posts,user(galleries)), includingnotToInclude("LEFT OUTER JOIN (")— matches theAuthor.cfc:8belongsTo(name="user", foreignKey="firstName")setup and the reported shape.crudSpec.cfc:1296asserts the #449 HABTM/throughgrouping is preserved formemberTeams(member)(ahasManybridge), guarding against undoing #449.
The issue references in the it() descriptions are correctly hash-escaped in the source.
Docs
Changelog fragment present and well-formed: changelog.d/3245-nested-include-flat-joins.fixed.md (<slug>.fixed.md). No direct CHANGELOG.md edit. The in-code comment block (sql.cfc:102-113) clearly documents why the metadata check exists.
Commits
fix(model): emit flat joins for belongsTo-chain nested includes (#3245) — valid type/scope, subject within 100 chars, not ALL-CAPS, conveys the "why."
Closes #3245
Problem
findAll(include="ParentModel(ChildModel)")regressed between Wheels 2 and 3: when the parenthesized intermediate is a plainbelongsToand the include mixes inner + outer joins,$fromClausewrapped the inner joins in a parenthesized group bound to theLEFT OUTER JOIN. That nested join expression scopes the rootFROMtable out of the inner join'sONclause, so MySQL rejects it with:The reporter's case:
Relationship belongsTo SecondaryContact belongsTo User, called asfindAll(include="RelationshipType,SecondaryContact(User)").Root cause
vendor/wheels/model/sql.cfc$fromClausederivedhasThroughAssociationpurely from a regex on the include string (^([^(]+)\(([^)]+)\)$). AnyIntermediate(Target)shape matched — including abelongsTochain — so the issue #449 HABTM/throughparenthesized-grouping heuristic over-fired. #449 introduced that grouping (commit6579f246c, July 2025), which is why this is absent in Wheels 2.Fix
Consult the association metadata instead of the string. When the single-paren pattern matches, look up the parenthesized intermediate association (
ListLastof the text before() invariables.wheels.class.associationsand only setneedsNestingwhen itstypeishasMany/hasOne— the OUTER-joined bridge the grouping was actually designed for. AbelongsTointermediate falls through to the existing flat-join branch (the Wheels 2 behavior).Before (bug) vs after, for
model("author").$fromClause(include="posts,user(galleries)"):The genuine HABTM/
throughcase (team→memberTeams(member), ahasManybridge) still nests:Tests
Two regression specs added to the directly-callable
$fromClauseharness invendor/wheels/tests/specs/model/crudSpec.cfc:throughbridge include asserts the parenthesized grouping is preserved (JOINs should be grouped #449 does not regress).Test status
Verified locally on SQLite (Lucee 7) and MySQL 9.7 (Lucee 7) — the engine in the report:
crudSpec163 pass / 0 fail / 0 error on both engines (includes both new regression specs).hasManyShortcutSpec(live HABTM/through/shortcut eager-load DB queries — JOINs should be grouped #449/model: hasMany shortcut= breaks the association entirely — $expandThroughAssociations misreads the shortcut through-chain as an include path (documented many-to-many pattern throws AssociationNotFound) #3109/hasMany shortcut association not recognised by include - "association not found" error #3208) 13/0/0 on both.wheels.tests.specs.modeldirectory 923/0/0 on SQLite.Cross-engine notes
Pure metadata read using
StructKeyExists/ListFindNoCase/ListLast/Mid— no closures, reserved scopes,attributeCollection=arguments,obj.map(), or barecfabort. The.typefield is set on every association (belongsTo/hasMany/hasOne). Behavior is adapter-independent (the change only decides flat vs nested join structure); MySQL was the engine in the report and is verified green.🤖 Generated with Claude Code