Repository navigation
fix(model): keep comma-bearing SQL functions intact in select and group by - #4637
Merged
Merged
Conversation
…up by #4634: with include=, the duplicate-prefix pass split the select clause with ListToArray, so an expression such as CONCAT(IF(ISNULL(titel.ausgabe)=1, '', CONCAT(titel.ausgabe, ' ')), kontakt.nachname) was split into fragments, the tail was treated as a duplicate property and dropped, and the generated SQL was invalid. Remove the second, duplicated inline copy of that pass in $createSQLFieldList and split both it and the distinct group-by item list with $splitOutsideFunctions(), which keeps function arguments together. SQL without comma-bearing functions is unchanged. Regression specs cover the reported expression shape, a plain select item before it, several comma-bearing functions side by side, and a distinct group-by that must not receive split subquery fragments. Signed-off-by: Peter Amiri <peter@alurium.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #4634.
select=expressions containing SQL functions with commas (CONCAT,IF,COALESCE,SUBSTRING, …) were corrupted when combined withinclude=(and the distinct group-by path), producing invalid SQL such as MySQL 1064.Two defects in
vendor/wheels/model/sql.cfm:$createSQLFieldListcalled$createSQLFieldListDuplicatePrefix()and then immediately ran a second, inline copy of the same 64-line body. Removed the redundant inline block.ListToArraysplit inside function arguments.$createSQLFieldListDuplicatePrefix()and the distinct branch of$groupByClause()split the clause on every comma, soCONCAT(IF(ISNULL(authors.firstname)=1, '', ...), authors.firstname) AS firstnamebecame fragments; the tail fragment was treated as a duplicate property (firstname) and dropped, truncating the select. Both now use the existing$splitOutsideFunctions(list, ","), which keeps parentheses groups together.$splitOutsideFunctions()already trims each item.SQL that does not contain comma-bearing functions is unchanged: the only difference for ordinary lists is that the helper skips empty segments (an empty select item was already a no-op through
ListAppend). The full model/database suites plus the engine matrix below cover that.Reproduction (before → after)
Model finder with
include="author"and:$selectClauseSELECT CONCAT(IF(ISNULL(authors.firstname)=1, '', CONCAT(authors.firstname, ' '))— tail, authors.firstname) AS firstnamedroppedSELECT CONCAT(IF(ISNULL(authors.firstname)=1, '', CONCAT(authors.firstname, ' ')), authors.firstname) AS firstname$selectClausewith a leadingidSELECT "c_o_r_e_posts"."id",CONCAT(IF(ISNULL(authors.firstname)=1, '', CONCAT(authors.firstname, ' '))$groupByClause(distinct=true)withid, (SELECT …) AS subnameGROUP BY "c_o_r_e_posts"."id", ' ') FROM c_o_r_e_authors)— split subquery fragments leakedGROUP BY "c_o_r_e_posts"."id"— subquery item skipped as designedRegression specs
New
vendor/wheels/tests/specs/model/CommaFunctionSelectSpec.cfc(4 specs), failing on the parent commit and passing with the fix:Verified failing-then-passing: with the production diff stashed, 0 passed / 4 failed; with the fix, 4 passed.
Tests
bash tools/test-local.sh model→ 1506 passedbash tools/test-local.sh database→ 189 passedtools/test-matrix.sh lucee7 sqlite,h2→ sqlite 7325 passed, h2 7292 passed; 0 failed / 0 erroredtools/test-matrix.sh lucee6,adobe2023,adobe2025,boxlang sqlite→ lucee6 7324, adobe2023 7319, adobe2025 7320, boxlang 7325; all 0 failed / 0 erroredbash tools/rustcfml/run-suite.sh→ RustCFML v0.741.0: 7297 passed, 0 failed, 0 errors, 182 skipped, 0 new vs baseline (the local SimpleLockRestartSpec flakes that usually show up on this host did not reproduce in this run)Changelog
changelog.d/4634-comma-sql-functions.fixed.md.Fixes #4634