Skip to content

Commit 0d50a92

Browse files
Peter Amiriclaude
andcommitted
fix(model): emit flat joins for belongsTo-chain nested includes (#3245)
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>
1 parent 7e65f61 commit 0d50a92

3 files changed

Lines changed: 62 additions & 3 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Nested `include` strings whose parenthesized intermediate is a `belongsTo` (e.g. `findAll(include="SecondaryContact(User)")`) again generate flat sibling joins, keeping the root `FROM` table in scope for every `ON` condition. The issue #449 HABTM/`through` parenthesized-grouping heuristic was over-firing on plain `belongsTo`-chain includes, producing a nested join expression that MySQL rejected with `Unknown column '<table>.<column>' in 'on clause'` — a regression from Wheels 2. The grouping now consults the association metadata and only nests for a genuine `hasMany`/`hasOne` bridge, so HABTM/`through` includes still nest as before (#3245)

vendor/wheels/model/sql.cfc

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,36 @@ component {
9999
local.hasThroughAssociation = false;
100100
local.iEnd = ArrayLen(local.associations);
101101

102-
// Check if this is specifically a through association pattern
102+
// Check if this is specifically a HABTM / through bridge pattern. The
103+
// parenthesized-INNER-join grouping below was added for issue #449 so a
104+
// many-to-many bridge (e.g. `memberTeams(member)`) keeps its nested inner
105+
// join scoped to the OUTER-joined bridge table. It must NOT fire for a plain
106+
// `belongsTo`-chain nested include (e.g. `SecondaryContact(User)`): there the
107+
// inner join's ON clause references the root FROM table, and wrapping it
108+
// inside the OUTER group scopes the root out — the MySQL "Unknown column ...
109+
// in 'on clause'" regression reported in issue #3245. So consult the actual
110+
// association metadata for the parenthesized intermediate instead of trusting
111+
// the include string alone: only a `hasMany` / `hasOne` intermediate (the
112+
// OUTER-joined bridge the grouping was designed for) qualifies; a `belongsTo`
113+
// intermediate falls through to the flat-join branch Wheels 2 emitted.
103114
local.originalInclude = Replace(arguments.include, " ", "", "all");
104115
if (Find("(", local.originalInclude)) {
105-
// Parse the include to see if it matches through pattern: intermediate(target)
116+
// Parse the include to see if it matches the pattern: intermediate(target)
106117
local.includePattern = ReFindNoCase("^([^(]+)\(([^)]+)\)$", local.originalInclude, 1, true);
107118
if (ArrayLen(local.includePattern.pos) >= 3) {
108-
local.hasThroughAssociation = true;
119+
// The association that parents the parenthesized target is the last
120+
// entry in the comma-list before the "(" (the only level this single-
121+
// paren pattern can match), so it is always a root-model association.
122+
local.intermediateName = ListLast(Mid(local.originalInclude, local.includePattern.pos[2], local.includePattern.len[2]));
123+
if (
124+
StructKeyExists(variables.wheels.class.associations, local.intermediateName)
125+
&& ListFindNoCase(
126+
"hasMany,hasOne",
127+
variables.wheels.class.associations[local.intermediateName].type
128+
)
129+
) {
130+
local.hasThroughAssociation = true;
131+
}
109132
}
110133
}
111134

vendor/wheels/tests/specs/model/crudSpec.cfc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,41 @@ component extends="wheels.WheelsTest" {
12621262

12631263
expect(actual).toBe("FROM #qi('c_o_r_e_authors')# USE INDEX(idx_authors_123) LEFT OUTER JOIN #qi('c_o_r_e_posts')# USE INDEX(idx_posts_123) ON #qi('c_o_r_e_authors')#.#qi('id')# = #qi('c_o_r_e_posts')#.#qi('authorid')# AND #qi('c_o_r_e_posts')#.#qi('deletedat')# IS NULL")
12641264
})
1265+
1266+
// Regression for issue #3245: a belongsTo-chain nested include
1267+
// (intermediate is `belongsTo`) mixed with an OUTER-joined sibling must
1268+
// emit FLAT sibling joins so the root FROM table stays in scope for every
1269+
// ON condition. Wheels 3 over-fired the issue #449 parenthesized grouping
1270+
// here, scoping the root out and triggering MySQL "Unknown column ... in
1271+
// 'on clause'". `author.user` is a belongsTo (inner) and `author.posts` /
1272+
// `user.galleries` are hasMany (outer) — exactly the reported shape.
1273+
it("emits flat joins for a belongsTo-chain nested include (issue ##3245)", () => {
1274+
actual = g.model("author").$fromClause(include = "posts,user(galleries)")
1275+
1276+
// the parenthesized intermediate (`user`) is a belongsTo, so NO grouping:
1277+
// every join sits at the top level and the root `authors` stays in scope.
1278+
expect(actual).notToInclude("LEFT OUTER JOIN (")
1279+
expect(actual).toBe(
1280+
"FROM #qi('c_o_r_e_authors')#"
1281+
& " LEFT OUTER JOIN #qi('c_o_r_e_posts')# ON #qi('c_o_r_e_authors')#.#qi('id')# = #qi('c_o_r_e_posts')#.#qi('authorid')# AND #qi('c_o_r_e_posts')#.#qi('deletedat')# IS NULL"
1282+
& " INNER JOIN #qi('c_o_r_e_users')# ON #qi('c_o_r_e_authors')#.#qi('firstname')# = #qi('c_o_r_e_users')#.#qi('firstname')#"
1283+
& " LEFT OUTER JOIN #qi('c_o_r_e_galleries')# ON #qi('c_o_r_e_users')#.#qi('id')# = #qi('c_o_r_e_galleries')#.#qi('userid')#"
1284+
)
1285+
})
1286+
1287+
// Regression for issue #449 (must NOT be undone by the #3245 fix): a genuine
1288+
// HABTM / `through` bridge nested include keeps the parenthesized grouping so
1289+
// the bridge's INNER join stays scoped to the OUTER-joined bridge table.
1290+
// Team.memberTeams is a hasMany (outer bridge); its nested `member` inner
1291+
// join references the bridge table, so the grouping is correct here.
1292+
it("preserves nested grouping for a HABTM/through bridge include (issue ##449)", () => {
1293+
actual = g.model("team").$fromClause(include = "memberTeams(member)")
1294+
1295+
expect(actual).toBe(
1296+
"FROM #qi('c_o_r_e_teams')#"
1297+
& " LEFT OUTER JOIN (#qi('c_o_r_e_memberteams')# INNER JOIN #qi('c_o_r_e_members')# ON #qi('c_o_r_e_memberteams')#.#qi('memberid')# = #qi('c_o_r_e_members')#.#qi('id')#) ON #qi('c_o_r_e_teams')#.#qi('id')# = #qi('c_o_r_e_memberteams')#.#qi('teamid')#"
1298+
)
1299+
})
12651300
})
12661301

12671302
describe("Tests that group", () => {

0 commit comments

Comments
 (0)