diff --git a/changelog.d/3245-nested-include-flat-joins.fixed.md b/changelog.d/3245-nested-include-flat-joins.fixed.md new file mode 100644 index 000000000..348842844 --- /dev/null +++ b/changelog.d/3245-nested-include-flat-joins.fixed.md @@ -0,0 +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 '.' 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) diff --git a/vendor/wheels/model/sql.cfc b/vendor/wheels/model/sql.cfc index 773068beb..48f4f5c50 100644 --- a/vendor/wheels/model/sql.cfc +++ b/vendor/wheels/model/sql.cfc @@ -99,13 +99,36 @@ component { local.hasThroughAssociation = false; local.iEnd = ArrayLen(local.associations); - // Check if this is specifically a through association pattern + // Check if this is specifically a HABTM / through bridge pattern. The + // parenthesized-INNER-join grouping below was added for issue #449 so a + // many-to-many bridge (e.g. `memberTeams(member)`) keeps its nested inner + // join scoped to the OUTER-joined bridge table. It must NOT fire for a plain + // `belongsTo`-chain nested include (e.g. `SecondaryContact(User)`): there the + // inner join's ON clause references the root FROM table, and wrapping it + // inside the OUTER group scopes the root out — the MySQL "Unknown column ... + // in 'on clause'" regression reported in issue #3245. So consult the actual + // association metadata for the parenthesized intermediate instead of trusting + // the include string alone: only a `hasMany` / `hasOne` intermediate (the + // OUTER-joined bridge the grouping was designed for) qualifies; a `belongsTo` + // intermediate falls through to the flat-join branch Wheels 2 emitted. local.originalInclude = Replace(arguments.include, " ", "", "all"); if (Find("(", local.originalInclude)) { - // Parse the include to see if it matches through pattern: intermediate(target) + // Parse the include to see if it matches the pattern: intermediate(target) local.includePattern = ReFindNoCase("^([^(]+)\(([^)]+)\)$", local.originalInclude, 1, true); if (ArrayLen(local.includePattern.pos) >= 3) { - local.hasThroughAssociation = true; + // The association that parents the parenthesized target is the last + // entry in the comma-list before the "(" (the only level this single- + // paren pattern can match), so it is always a root-model association. + local.intermediateName = ListLast(Mid(local.originalInclude, local.includePattern.pos[2], local.includePattern.len[2])); + if ( + StructKeyExists(variables.wheels.class.associations, local.intermediateName) + && ListFindNoCase( + "hasMany,hasOne", + variables.wheels.class.associations[local.intermediateName].type + ) + ) { + local.hasThroughAssociation = true; + } } } diff --git a/vendor/wheels/tests/specs/model/crudSpec.cfc b/vendor/wheels/tests/specs/model/crudSpec.cfc index a151074d8..da1e217ee 100644 --- a/vendor/wheels/tests/specs/model/crudSpec.cfc +++ b/vendor/wheels/tests/specs/model/crudSpec.cfc @@ -1262,6 +1262,41 @@ component extends="wheels.WheelsTest" { 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") }) + + // Regression for issue #3245: a belongsTo-chain nested include + // (intermediate is `belongsTo`) mixed with an OUTER-joined sibling must + // emit FLAT sibling joins so the root FROM table stays in scope for every + // ON condition. Wheels 3 over-fired the issue #449 parenthesized grouping + // here, scoping the root out and triggering MySQL "Unknown column ... in + // 'on clause'". `author.user` is a belongsTo (inner) and `author.posts` / + // `user.galleries` are hasMany (outer) — exactly the reported shape. + it("emits flat joins for a belongsTo-chain nested include (issue ##3245)", () => { + actual = g.model("author").$fromClause(include = "posts,user(galleries)") + + // the parenthesized intermediate (`user`) is a belongsTo, so NO grouping: + // every join sits at the top level and the root `authors` stays in scope. + expect(actual).notToInclude("LEFT OUTER JOIN (") + expect(actual).toBe( + "FROM #qi('c_o_r_e_authors')#" + & " 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" + & " 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')#" + & " 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')#" + ) + }) + + // Regression for issue #449 (must NOT be undone by the #3245 fix): a genuine + // HABTM / `through` bridge nested include keeps the parenthesized grouping so + // the bridge's INNER join stays scoped to the OUTER-joined bridge table. + // Team.memberTeams is a hasMany (outer bridge); its nested `member` inner + // join references the bridge table, so the grouping is correct here. + it("preserves nested grouping for a HABTM/through bridge include (issue ##449)", () => { + actual = g.model("team").$fromClause(include = "memberTeams(member)") + + expect(actual).toBe( + "FROM #qi('c_o_r_e_teams')#" + & " 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')#" + ) + }) }) describe("Tests that group", () => {