From 0222f58be0457a6129fea2788411a0b682994478 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 3 Aug 2026 22:00:32 -0700 Subject: [PATCH 1/2] fix(model): scope a nested include's inner join to its own parent join MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findAll(include="...")` copied every INNER JOIN into every LEFT OUTER JOIN. With a single outer join — the shape both issue #449 and issue #3245 exercise — that is indistinguishable from correct. Add a shallow sibling ahead of the nested group and it fans out: include = "c_o_r_e_comments,classifications(tag)" FROM posts LEFT OUTER JOIN (comments INNER JOIN tags ON classifications.tagid = tags.id) ON ... LEFT OUTER JOIN (classifications INNER JOIN tags ON classifications.tagid = tags.id) ON ... The first group references `classifications` before the query introduces it. Reported against Oracle as ORA-00904, but it is engine-independent: on SQLite the same include errors with `no such column: c_o_r_e_classifications.tagid`. The information needed to place each join correctly already existed and was being thrown away. `$expandedAssociations` walks the include with a `levels` stack, so it knows exactly which association nests under which — and then returned a flat array, leaving `$fromClause` to re-derive parentage by regex over the generated SQL text. Both prior patches to this code fought that same information loss. Each entry now carries `parentPosition`, and an INNER join is grouped with one OUTER join: the association it is actually nested under. A root-level INNER join has no enclosing group and stays flat, which keeps the root FROM table in scope for its ON condition — the #3245 failure mode, now prevented structurally rather than by a gate. That also fixes a root-level INNER join being dropped from the output entirely. The old loop only ever emitted the outer-join array, so `include="author,classifications(tag)"` never emitted `INNER JOIN authors` at top level at all — it appeared only inside the classifications group, where its ON references the out-of-scope root. The gate is now redundant, so it is gone. It tested the include STRING against `^([^(]+)\(([^)]+)\)$`, which only matches when the nested group comes last, so `a(b),c` and `c,a(b)` generated different SQL for the same query. Whether a join is correctly scoped is a property of the association tree, not of where the user typed the parentheses. Removing it also collapses the duplicated flat-join branch: 200 lines changed, 42 fewer. Behaviour change, called out in the changelog: in the nested-first form the nested INNER JOIN used to be emitted at the root, demoting the sibling LEFT OUTER JOIN to an inner join and silently dropping parent rows with no associated record. On the fixtures that is 3 rows where the nested-last form returns 15. Both orderings now return 15. A query written nested-first can therefore return more rows than before — the rows a hasMany/hasOne include exists to preserve. Apps that relied on the filtering should set joinType="inner". Red-first, with sql.cfc reverted to develop: 945 pass / 3 fail / 1 error. The three string assertions fail on the generated SQL and the executable one errors with the SQLite message above — the reported symptom, reproduced. Verification, lucee7 + sqlite, full core suite, back to back on one machine: develop ab901cff7 4732 pass / 0 fail / 0 error this branch 4736 pass / 0 fail / 0 error Exactly +4, the new specs. The existing #449 and #3245 regression specs pass unchanged and byte-identically — neither expected string needed editing. Reported by Mike Grogan, who also submitted a working patch. His fix localises the same loop but decides ownership by substring-searching the outer join's table name inside each inner join string, which misfires when one table name contains another (c_o_r_e_photos inside c_o_r_e_photogalleryphotos, already present in these fixtures) and leaves the ordering inconsistency in place. Closes #3334 Signed-off-by: Peter Amiri --- .../3334-include-join-scoping.fixed.md | 2 + vendor/wheels/model/sql.cfc | 200 ++++++++---------- vendor/wheels/tests/specs/model/crudSpec.cfc | 71 ++++++- 3 files changed, 160 insertions(+), 113 deletions(-) create mode 100644 changelog.d/3334-include-join-scoping.fixed.md diff --git a/changelog.d/3334-include-join-scoping.fixed.md b/changelog.d/3334-include-join-scoping.fixed.md new file mode 100644 index 000000000..6536917b9 --- /dev/null +++ b/changelog.d/3334-include-join-scoping.fixed.md @@ -0,0 +1,2 @@ +- `findAll(include="...")` no longer copies a nested association's `INNER JOIN` into unrelated sibling joins. When a nested group followed one or more shallow associations (e.g. `include="comments,classifications(tag)"`), the issue #449 parenthesized grouping spliced the nested `INNER JOIN` into every preceding `LEFT OUTER JOIN`, so those joins referenced a table the query had not introduced yet — Oracle rejected it with `ORA-00904: invalid identifier`, MySQL with `Unknown column '.' in 'on clause'`. Each `INNER JOIN` is now scoped to the single association it is nested under, taken from the include's association tree rather than re-derived from the generated SQL text. Reported with a working patch by Mike Grogan (#3334) +- **Behaviour change:** `include` order no longer changes the SQL a query generates. Grouping used to be gated on an anchored pattern over the include string that only matched when the nested group came last, so `include="a(b),c"` and `include="c,a(b)"` produced structurally different joins for the same query. In the nested-first form the nested `INNER JOIN` was emitted at the root, which demoted the sibling `LEFT OUTER JOIN` to an inner join and silently dropped parent rows that had no associated record. Both orderings now emit the same joins, so a query written in the nested-first form can return **more** rows than before — the rows a `hasMany`/`hasOne` include is meant to preserve. Pass `joinType="inner"` on the association if the filtering was intentional (#3334) diff --git a/vendor/wheels/model/sql.cfc b/vendor/wheels/model/sql.cfc index 0d6b945ef..c6d9a3478 100644 --- a/vendor/wheels/model/sql.cfc +++ b/vendor/wheels/model/sql.cfc @@ -92,134 +92,97 @@ component { includeSoftDeletes = arguments.includeSoftDeletes ); - // Check if we need to nest inner joins (when both inner and outer joins are present) - // Only apply nesting for HABTM patterns, not for all mixed join scenarios - local.hasInnerJoins = false; - local.hasOuterJoins = false; - local.hasThroughAssociation = false; local.iEnd = ArrayLen(local.associations); - - // 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 the pattern: intermediate(target) - local.includePattern = ReFindNoCase("^([^(]+)\(([^)]+)\)$", local.originalInclude, 1, true); - if (ArrayLen(local.includePattern.pos) >= 3) { - // 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; - } + + // Build the join statements. Every association carries the position of the + // association it is nested under (`parentPosition`, 0 at the root), so the + // grouping decision below reads the include structure instead of re-deriving + // it from the generated SQL text. + // + // This replaces a gate that only grouped when the include string matched + // `^([^(]+)\(([^)]+)\)$` — i.e. only when the nested group came LAST. Whether + // a join is scoped correctly is a property of the association tree, not of + // where the user happened to type the parentheses, and the old anchored + // pattern made `a(b),c` and `c,a(b)` generate different SQL for the same query + // (issue #3334). + local.joins = []; + + for (local.i = 1; local.i <= local.iEnd; local.i++) { + local.indexHint = this.$indexHint( + useIndex = arguments.useIndex, + modelName = local.associations[local.i].modelName, + adapterName = arguments.adapterName + ); + local.join = local.associations[local.i].join; + if (Len(local.indexHint)) { + // replace the quoted table name with the quoted table name & index hint + // TODO: factor in table aliases.. the index hint is placed after the table alias + local.quotedAssocTable = variables.wheels.class.adapter.$quoteIdentifier(local.associations[local.i].tableName); + local.join = Replace( + local.join, + " #local.quotedAssocTable# ", + " #local.quotedAssocTable# #local.indexHint# ", + "one" + ); } + local.joins[local.i] = local.join; } - + + // Decide which INNER joins get pulled inside a parenthesized group. An INNER join + // belongs to exactly one OUTER join — the association it is nested under in the + // include string — and must never be copied into a sibling, which would reference + // a table the query has not introduced yet (issue #3334: ORA-00904 / MySQL + // "unknown column in on clause"). Prior to this the loop appended every INNER join + // to every OUTER join, which only looked correct because issues #449 and #3245 both + // exercise a single OUTER join. A root-level INNER join (`parentPosition` 0) has no + // enclosing group and stays flat, keeping the root FROM table in scope for its ON. + local.nestedJoins = {}; + local.isNested = {}; for (local.i = 1; local.i <= local.iEnd; local.i++) { - if (FindNoCase("INNER", local.associations[local.i].join)) { - local.hasInnerJoins = true; - } - if (FindNoCase("OUTER", local.associations[local.i].join) || FindNoCase("LEFT", local.associations[local.i].join)) { - local.hasOuterJoins = true; + local.parentPosition = StructKeyExists(local.associations[local.i], "parentPosition") + ? local.associations[local.i].parentPosition + : 0; + if ( + FindNoCase("INNER", local.joins[local.i]) + && local.parentPosition > 0 + && !FindNoCase("INNER", local.joins[local.parentPosition]) + ) { + if (!StructKeyExists(local.nestedJoins, local.parentPosition)) { + local.nestedJoins[local.parentPosition] = []; + } + ArrayAppend(local.nestedJoins[local.parentPosition], local.joins[local.i]); + local.isNested[local.i] = true; } } - - // Only apply nesting for through associations with mixed join types - local.needsNesting = local.hasInnerJoins && local.hasOuterJoins && local.hasThroughAssociation; - // build the join statements - if (local.needsNesting) { - // group inner joins with parentheses and outer joins separately - local.innerJoins = []; - local.outerJoins = []; + for (local.i = 1; local.i <= local.iEnd; local.i++) { + if (!StructKeyExists(local.isNested, local.i)) { + local.join = local.joins[local.i]; - for (local.i = 1; local.i <= local.iEnd; local.i++) { - local.indexHint = this.$indexHint( - useIndex = arguments.useIndex, - modelName = local.associations[local.i].modelName, - adapterName = arguments.adapterName - ); - local.join = local.associations[local.i].join; - if (Len(local.indexHint)) { - // replace the quoted table name with the quoted table name & index hint - // TODO: factor in table aliases.. the index hint is placed after the table alias - local.quotedAssocTable = variables.wheels.class.adapter.$quoteIdentifier(local.associations[local.i].tableName); - local.join = Replace( - local.join, - " #local.quotedAssocTable# ", - " #local.quotedAssocTable# #local.indexHint# ", - "one" - ); - } - - if (FindNoCase("INNER", local.join)) { - ArrayAppend(local.innerJoins, local.join); - } else { - ArrayAppend(local.outerJoins, local.join); - } - } - - for (local.i = 1; local.i <= ArrayLen(local.outerJoins); local.i++) { - local.outerJoin = local.outerJoins[local.i]; - - // If we have inner joins, we need to group them in the outer join - if (ArrayLen(local.innerJoins) > 0) { + if (StructKeyExists(local.nestedJoins, local.i)) { // Find the table being joined in the outer join - local.joinTableMatch = ReFindNoCase("LEFT OUTER JOIN ([^\s]+)", local.outerJoin, 1, true); + local.joinTableMatch = ReFindNoCase("LEFT OUTER JOIN ([^\s]+)", local.join, 1, true); if (ArrayLen(local.joinTableMatch.pos) >= 2 && local.joinTableMatch.pos[2] > 0) { - local.joinTable = Mid(local.outerJoin, local.joinTableMatch.pos[2], local.joinTableMatch.len[2]); - + local.joinTable = Mid(local.join, local.joinTableMatch.pos[2], local.joinTableMatch.len[2]); + // Build grouped inner joins: (subscriptions INNER JOIN magazines ON ...) local.groupedInner = "(" & local.joinTable; - for (local.j = 1; local.j <= ArrayLen(local.innerJoins); local.j++) { - local.groupedInner &= " " & local.innerJoins[local.j]; + local.jEnd = ArrayLen(local.nestedJoins[local.i]); + for (local.j = 1; local.j <= local.jEnd; local.j++) { + local.groupedInner &= " " & local.nestedJoins[local.i][local.j]; } local.groupedInner &= ")"; - + // Replace in the outer join - local.outerJoin = Replace(local.outerJoin, "LEFT OUTER JOIN " & local.joinTable, "LEFT OUTER JOIN " & local.groupedInner); + local.join = Replace( + local.join, + "LEFT OUTER JOIN " & local.joinTable, + "LEFT OUTER JOIN " & local.groupedInner, + "one" + ); } } - - local.rv = ListAppend(local.rv, local.outerJoin, " "); - } - } else { - // original logic for when nesting is not needed - for (local.i = 1; local.i <= local.iEnd; local.i++) { - local.indexHint = this.$indexHint( - useIndex = arguments.useIndex, - modelName = local.associations[local.i].modelName, - adapterName = arguments.adapterName - ); - local.join = local.associations[local.i].join; - if (Len(local.indexHint)) { - // replace the quoted table name with the quoted table name & index hint - // TODO: factor in table aliases.. the index hint is placed after the table alias - local.quotedAssocTable = variables.wheels.class.adapter.$quoteIdentifier(local.associations[local.i].tableName); - local.join = Replace( - local.join, - " #local.quotedAssocTable# ", - " #local.quotedAssocTable# #local.indexHint# ", - "one" - ); - } + local.rv = ListAppend(local.rv, local.join, " "); } } @@ -1327,6 +1290,12 @@ component { // add the current class name so that the levels list start at the lowest level local.levels = variables.wheels.class.modelName; + // mirrors `local.levels` with the position in `local.rv` of the association that + // opened each level, so every entry can record the association it nests under. + // Callers that group joins (see `$fromClause`) would otherwise have to re-derive + // parentage from the generated SQL text — the regex guesswork behind issue #3334. + local.parentPositions = []; + // expand through associations before processing local.include = $expandThroughAssociations(arguments.include); @@ -1342,6 +1311,9 @@ component { local.pos = 1; for (local.i = 1; local.i <= local.iEnd; local.i++) { + // the association that opened the level we are currently inside, or 0 at the root + local.parentPosition = ArrayLen(local.parentPositions) ? local.parentPositions[ArrayLen(local.parentPositions)] : 0; + // look for the next delimiter sequence in the string and set it (can be single delims or a chain, e.g ',' or ')),' local.delimFind = ReFind("[(\(|\)|,)]+", local.include, local.pos, true); local.delimSequence = Mid(local.include, local.delimFind.pos[1], local.delimFind.len[1]); @@ -1553,8 +1525,12 @@ component { local.delimChar = Mid(local.delimSequence, local.j, 1); if (local.delimChar == "(") { local.levels = ListAppend(local.levels, local.classAssociations[local.name].modelName); + // this association parents everything inside the parentheses it just opened; + // `local.i` is its position in `local.rv` because we append exactly once per pass + ArrayAppend(local.parentPositions, local.i); } else if (local.delimChar == ")") { local.levels = ListDeleteAt(local.levels, ListLen(local.levels)); + ArrayDeleteAt(local.parentPositions, ArrayLen(local.parentPositions)); } } @@ -1572,6 +1548,8 @@ component { // identifiers contain the ON substring (e.g. uppercase H2 schemas) local.onPos = Find(" ON ", local.entry.join); local.entry.joinOnConditions = local.onPos GT 0 ? Mid(local.entry.join, local.onPos + 4, Len(local.entry.join)) : ""; + // position in this array of the association this one is nested under (0 = root level) + local.entry.parentPosition = local.parentPosition; ArrayAppend(local.rv, local.entry); } return local.rv; diff --git a/vendor/wheels/tests/specs/model/crudSpec.cfc b/vendor/wheels/tests/specs/model/crudSpec.cfc index 96caa4604..8de2f0fb1 100644 --- a/vendor/wheels/tests/specs/model/crudSpec.cfc +++ b/vendor/wheels/tests/specs/model/crudSpec.cfc @@ -1273,8 +1273,10 @@ component extends="wheels.WheelsTest" { 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. + // No join here qualifies for grouping: `user` is INNER but sits at the root + // (nothing encloses it, and its ON references the root `authors`), and + // `galleries` is OUTER. Every join stays at the top level, so the root table + // remains in scope for every ON condition. expect(actual).notToInclude("LEFT OUTER JOIN (") expect(actual).toBe( "FROM #qi('c_o_r_e_authors')#" @@ -1297,6 +1299,71 @@ component extends="wheels.WheelsTest" { & " 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')#" ) }) + + // Regression for issue #3334: the issue #449 grouping copied EVERY inner join + // into EVERY outer join, so a shallow sibling listed before the nested group + // got the nested group's INNER join spliced into it — referencing a table the + // query has not introduced yet (ORA-00904 / "unknown column in on clause"). + // `Post.c_o_r_e_comments` and `Post.classifications` are both hasMany (outer); + // `Classification.tag` is a belongsTo (inner) whose ON clause references + // `classifications`, so it belongs to the classifications group and nowhere else. + it("scopes a nested inner join to its own parent, not to every outer join (issue ##3334)", () => { + actual = g.model("post").$fromClause(include = "c_o_r_e_comments,classifications(tag)") + + // the comments join must stay flat — nothing from the classifications + // subtree may appear inside it + expect(actual).toBe( + "FROM #qi('c_o_r_e_posts')#" + & " LEFT OUTER JOIN #qi('c_o_r_e_comments')# ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_comments')#.#qi('postid')#" + & " LEFT OUTER JOIN (#qi('c_o_r_e_classifications')# INNER JOIN #qi('c_o_r_e_tags')# ON #qi('c_o_r_e_classifications')#.#qi('tagid')# = #qi('c_o_r_e_tags')#.#qi('id')#) ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_classifications')#.#qi('postid')#" + ) + }) + + // Second shape of issue #3334, and a residual case of issue #3245 that the + // #3245 gate does not cover: a ROOT-level inner join (`Post.author` is a + // belongsTo) alongside a nested group. Its ON clause references the root + // `posts` table, so pulling it inside the classifications parentheses scopes + // the root out — the same "unknown column in on clause" failure #3245 fixed + // for the flat branch. A root-level join has no enclosing group; it stays flat. + it("keeps a root-level inner join out of the nested group (issue ##3334)", () => { + actual = g.model("post").$fromClause(include = "author,classifications(tag)") + + expect(actual).toBe( + "FROM #qi('c_o_r_e_posts')#" + & " INNER JOIN #qi('c_o_r_e_authors')# ON #qi('c_o_r_e_posts')#.#qi('authorid')# = #qi('c_o_r_e_authors')#.#qi('id')#" + & " LEFT OUTER JOIN (#qi('c_o_r_e_classifications')# INNER JOIN #qi('c_o_r_e_tags')# ON #qi('c_o_r_e_classifications')#.#qi('tagid')# = #qi('c_o_r_e_tags')#.#qi('id')#) ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_classifications')#.#qi('postid')#" + ) + }) + + // Issue #3334, the reporter's actual complaint: the grouping used to be gated on + // an anchored regex over the include STRING, so it only fired when the nested + // group came last. `a(b),c` and `c,a(b)` therefore generated structurally + // different SQL for the same query — one of them invalid. Grouping is now decided + // from the association tree, so include order only reorders the emitted joins. + it("emits the same joins wherever the nested group sits in the include (issue ##3334)", () => { + comments = " LEFT OUTER JOIN #qi('c_o_r_e_comments')# ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_comments')#.#qi('postid')#" + classifications = " LEFT OUTER JOIN (#qi('c_o_r_e_classifications')# INNER JOIN #qi('c_o_r_e_tags')# ON #qi('c_o_r_e_classifications')#.#qi('tagid')# = #qi('c_o_r_e_tags')#.#qi('id')#) ON #qi('c_o_r_e_posts')#.#qi('id')# = #qi('c_o_r_e_classifications')#.#qi('postid')#" + + expect(g.model("post").$fromClause(include = "c_o_r_e_comments,classifications(tag)")).toBe( + "FROM #qi('c_o_r_e_posts')#" & comments & classifications + ) + expect(g.model("post").$fromClause(include = "classifications(tag),c_o_r_e_comments")).toBe( + "FROM #qi('c_o_r_e_posts')#" & classifications & comments + ) + }) + + // Executable form of the above — the string assertions pin the SQL, this proves + // the database accepts it and that both orderings agree on the result set. The + // nested-first ordering used to emit `tags` as a ROOT-level inner join, which + // silently demoted the outer join to an inner one and dropped every post with no + // classification; it now keeps them, matching the nested-last ordering. + it("returns the same rows wherever the nested group sits in the include (issue ##3334)", () => { + nestedLast = g.model("post").findAll(include = "c_o_r_e_comments,classifications(tag)") + nestedFirst = g.model("post").findAll(include = "classifications(tag),c_o_r_e_comments") + + expect(nestedLast.recordCount).toBeGT(0) + expect(nestedLast.recordCount).toBe(nestedFirst.recordCount) + }) }) describe("Tests that group", () => { From b9d452a2d7d4dd2cd079a8fb4bb75904101d0de7 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Mon, 3 Aug 2026 23:50:13 -0700 Subject: [PATCH 2/2] fix(model): tolerate an unbalanced include the way the level walker always did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on a clean-context re-read of this branch, not by a failing test. `$expandedAssociations` walks the include's delimiter characters and pops a level on every `)`. The new `parentPositions` stack pops alongside it — and where `ListDeleteAt` on a one-element list quietly tolerates an unbalanced include like `"c_o_r_e_comments)"`, `ArrayDeleteAt(x, 0)` throws `can not remove Element at position [0]`. Develop resolves that input to a plain join and ignores the stray paren. This branch would have started erroring on it — a new failure mode on user input, introduced by a change that has nothing to do with malformed includes. Guarded, and pinned by a spec that reproduces it: with the guard removed the suite goes to 949 pass / 1 error with that exact message. lucee7 + sqlite, full core suite: 4737 pass / 0 fail / 0 error — develop 4732 plus the five specs this branch now adds. Signed-off-by: Peter Amiri --- vendor/wheels/model/sql.cfc | 8 +++++++- vendor/wheels/tests/specs/model/crudSpec.cfc | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/vendor/wheels/model/sql.cfc b/vendor/wheels/model/sql.cfc index c6d9a3478..9799d565e 100644 --- a/vendor/wheels/model/sql.cfc +++ b/vendor/wheels/model/sql.cfc @@ -1530,7 +1530,13 @@ component { ArrayAppend(local.parentPositions, local.i); } else if (local.delimChar == ")") { local.levels = ListDeleteAt(local.levels, ListLen(local.levels)); - ArrayDeleteAt(local.parentPositions, ArrayLen(local.parentPositions)); + // Guarded because an unbalanced include (`"posts)"`) reaches here with an + // empty stack, and ArrayDeleteAt(x, 0) throws where the ListDeleteAt above + // quietly tolerates it. Malformed includes behaved as before this change; + // they should not start erroring differently because of it. + if (ArrayLen(local.parentPositions)) { + ArrayDeleteAt(local.parentPositions, ArrayLen(local.parentPositions)); + } } } diff --git a/vendor/wheels/tests/specs/model/crudSpec.cfc b/vendor/wheels/tests/specs/model/crudSpec.cfc index 8de2f0fb1..1b66e6ef7 100644 --- a/vendor/wheels/tests/specs/model/crudSpec.cfc +++ b/vendor/wheels/tests/specs/model/crudSpec.cfc @@ -1335,6 +1335,17 @@ component extends="wheels.WheelsTest" { ) }) + // An unbalanced include reaches the level-tracking loop with an empty parent + // stack. `ListDeleteAt` on a one-element list tolerates that; `ArrayDeleteAt(x, 0)` + // throws. Malformed includes resolved to a plain join before the issue #3334 + // change and must keep doing so — the fix must not turn a tolerated input into a + // new error. + it("tolerates an unbalanced include the way it always did (issue ##3334)", () => { + actual = g.model("post").$fromClause(include = "c_o_r_e_comments)") + + expect(actual).toInclude(qi("c_o_r_e_comments")) + }) + // Issue #3334, the reporter's actual complaint: the grouping used to be gated on // an anchored regex over the include STRING, so it only fired when the nested // group came last. `a(b),c` and `c,a(b)` therefore generated structurally