fix(model): quote column identifiers in SELECT clause builder#2787
Conversation
The WHERE and ORDER BY clause builders already routed column names through the adapter's $quoteIdentifier, but $createSQLFieldList — the SELECT/GROUP BY engine — appended the column part raw. Models backed by tables with reserved-word column names (e.g. `key`, `order`, `group`) blew up on `findAll`/`findOne`/dynamic finders with cryptic SQL syntax errors as soon as the SELECT list mentioned the column. Also strips quote chars from the property extracted by the duplicate-column rename loop so the alias replacement still matches the unquoted ` AS <alias>` form, and updates the empty-pagination columnList extraction in read.cfc to strip identifier quotes before stripping the table prefix. Fixes #2784 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…s in models guide Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a correct, well-scoped fix that extends column-identifier quoting from the WHERE/ORDER BY builders (which already called $quoteIdentifier) to the SELECT/GROUP BY builder ($createSQLFieldList) and the empty-pagination QueryNew path in read.cfc. The implementation is sound, the commit is clean, and the new spec covers the two primary code paths. A few nits follow; none are blocking.
Correctness
The fix is correct. The core change at vendor/wheels/model/sql.cfc:570,572 mirrors the pattern already used by WHERE/ORDER BY builders:
// Before
local.toAppend &= local.iItem;
// ...
local.toAppend &= local.classData.properties[local.iItem].column;
// After
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.iItem);
// ...
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.classData.properties[local.iItem].column);The downstream $stripIdentifierQuotes call at sql.cfc:638 and the corresponding regex simplification in read.cfc:222 are necessary and correct adaptations.
One path that is correct but worth a note: the aliasFound branch at sql.cfc:677 was also changed to add $quoteIdentifier:
local.newItem = local.alias & "." & variables.wheels.class.adapter.$quoteIdentifier(local.property) & " AS " & local.newProperty;At that point local.property is the unquoted column name (extracted via SpanExcluding, then stripped by $stripIdentifierQuotes at line 638). Re-quoting it here is correct. However, this branch is not exercised by the new spec (it only fires when a JOIN alias exactly equals the model's pluralized name — a narrow case). Not a blocker, but worth noting.
Conventions
Multi-line comment blocks — read.cfc:217-220 and sql.cfc:634-637 both add 4-line block comments. Per CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The WHY is genuinely non-obvious here (the strip is needed because the regex only handles bare identifiers), so a comment is justified, but it should be condensed to a single line:
vendor/wheels/model/read.cfc:217-220
// Before regex: $createSQLFieldList now quotes identifiers; strip them so the bare-identifier regex below works.
local.columns = variables.wheels.class.adapter.$stripIdentifierQuotes(local.columns);vendor/wheels/model/sql.cfc:634-637
// $quoteIdentifier is now applied to column names above; strip before alias matching.
local.property = variables.wheels.class.adapter.$stripIdentifierQuotes(local.property);Tests
Happy path and cross-engine idiom — The three specs in reservedColumnQuotingSpec.cfc use m.$quoteColumn(...) to build the expected value dynamically against the live adapter, making the assertions dialect-agnostic. That is exactly the right pattern.
$quoteColumn used for a table name (reservedColumnQuotingSpec.cfc:15,27,43):
var qTable = m.$quoteColumn("c_o_r_e_cities");$quoteColumn is defined as delegating to $quoteIdentifier, which is also what table-name quoting uses, so this is functionally correct. But the method name suggests a column, not a table. A future reader may be confused. Prefer m.$quoteIdentifier("c_o_r_e_cities") directly (calling the adapter method), or document why $quoteColumn is used. Minor, but slightly misleading.
Missing coverage for read.cfc:221 — The $stripIdentifierQuotes + regex path at read.cfc:221-224 (the QueryNew path for zero-row paginated results) is not exercised by the new spec. It is indirectly validated if any existing pagination test returns zero results, but there is no explicit test that hits this branch with a reserved-word column. Consider adding an it block in reservedColumnQuotingSpec.cfc (or crudSpec.cfc) that calls findAll(page=1, perPage=25) on a model with a reserved-word column name and verifies the returned query's columnList is well-formed when the count is zero.
Docs
CHANGELOG entry under [Unreleased] ✓ — clear, accurate, references the affected functions.
Commits
fix(model): quote column identifiers in SELECT clause builder
- Type
fix✓ - Scope
model✓ - Subject ≤ 100 chars ✓, not ALL-CAPS ✓
- Body explains the why (inconsistency between WHERE/ORDER BY vs SELECT) ✓
Fixes #2784closes the upstream issue ✓- DCO
Signed-off-bytrailer present and matching git author ✓
Security
No concerns. All inputs to $quoteIdentifier are model-metadata property/column names, never user-supplied strings.
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR correctly extends column-identifier quoting from the WHERE/ORDER BY builders to the SELECT/GROUP BY builder and the empty-pagination QueryNew path, fixing reserved-word column crashes on every supported dialect. The two-commit stack is sound. A new documentation commit was added since the previous review pass; the docs are accurate but one omission may mislead readers. The remaining findings are all nits — none are blocking, verdict is comment.
Correctness
The implementation is correct. Core changes at sql.cfc:570,572:
// Before
local.toAppend &= local.iItem;
local.toAppend &= local.classData.properties[local.iItem].column;
// After
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.iItem);
local.toAppend &= variables.wheels.class.adapter.$quoteIdentifier(local.classData.properties[local.iItem].column);The downstream $stripIdentifierQuotes calls at sql.cfc:638 and read.cfc:221 are necessary adaptations so that the alias-replacement regex and the empty-pagination QueryNew column list continue to operate on bare identifiers. The aliasFound branch at sql.cfc:677 re-quotes the stripped property correctly.
Conventions
Multi-line comment blocks — read.cfc:217-220 and sql.cfc:634-637 each contain a 4-line block comment. Per CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The WHY is non-obvious here, so a comment is justified; it just needs to be condensed. Suggested single-line replacements:
vendor/wheels/model/read.cfc:217
// Strip dialect quotes: $createSQLFieldList now quotes identifiers; the bare-identifier regex below requires unquoted input.
local.columns = variables.wheels.class.adapter.$stripIdentifierQuotes(local.columns);vendor/wheels/model/sql.cfc:634
// Strip dialect quotes added above so alias matching and downstream concatenation work on bare identifiers.
local.property = variables.wheels.class.adapter.$stripIdentifierQuotes(local.property);Tests
$quoteColumn used for a table name (reservedColumnQuotingSpec.cfc:15,27,43):
var qTable = m.$quoteColumn("c_o_r_e_cities");$quoteColumn delegates to $quoteIdentifier so this is functionally correct, but the method name implies a column, not a table. Consider calling variables.wheels.class.adapter.$quoteIdentifier("c_o_r_e_cities") directly (matching how crudSpec.cfc accesses it via the qi() helper) or at minimum add a brief comment explaining why the column helper is used for the table identifier here.
Missing coverage for the zero-row pagination path — read.cfc:221-225 (the QueryNew branch reached when a paginated query returns zero rows) is not exercised by the new spec. Any reserved-word column silently bypasses this path in the current tests. Consider adding an it block in reservedColumnQuotingSpec.cfc (or the existing crudSpec.cfc pagination section) that calls findAll(page=1, perPage=25) on a model with a reserved-word column name and asserts the returned query's columnList is well-formed when the result set is empty.
Docs
ORDER BY omitted from the new guide note (web/sites/guides/src/content/docs/v4-0-1-snapshot/basics/models-and-the-orm.mdx:54):
Wheels quotes all column identifiers in SELECT and GROUP BY, so the alias stays clean while the generated SQL brackets the underlying column name for the target dialect.
ORDER BY was already routing column names through $quoteIdentifier before this fix. Saying only "SELECT and GROUP BY" may leave readers uncertain whether ORDER BY is also safe. Suggested wording:
Wheels quotes all column identifiers in SELECT, GROUP BY, and ORDER BY clauses, so the alias stays clean while the generated SQL brackets the underlying column name for the target dialect.
CHANGELOG — entry under [Unreleased] is accurate and clearly scoped.
Commits
Commit 1 — fix(model): quote column identifiers in SELECT clause builder
- Type
fix, scopemodel, subject <= 100 chars, not ALL-CAPS - Body explains the why (inconsistency between WHERE/ORDER BY vs SELECT)
Fixes #2784, DCOSigned-off-bytrailer present and matchingclaude[bot]identity
Commit 2 — docs(web/guides): note reserved-word column support via property alias in models guide
- Type
docs, scopeweb/guides, subject <= 100 chars, not ALL-CAPS - DCO
Signed-off-bytrailer present - Correctly targets
v4-0-1-snapshotrather thanv4-0-0(the fix is 4.0.1-only; the alias workaround would not have prevented the SELECT crash on 4.0.0)
Security
No concerns. All inputs to $quoteIdentifier are model-metadata property/column names, not user-supplied strings.
Wheels Bot — Reviewer B (round 1)A's review is accurate and well-grounded in the diff. The correctness analysis, nit identification, and verdict are defensible. No sycophancy. One process observation and a minor line-attribution imprecision follow; no new blocking issues found. SycophancyNone detected. A's "correct, well-scoped fix" claim is backed by specific code citations — the WHERE/ORDER BY parity argument and the per-branch analysis of False positivesOne minor imprecision on line attribution: A cites `reservedColumnQuotingSpec.cfc:15,27,43` for the `$quoteColumn`-as-table-name calls. From the diff, the first `$quoteColumn("c_o_r_e_cities")` sits at line 16 (the line immediately after `var actual = m.$selectClause(...)` at line 15), the second at line 29, and the third at approximately line 46. Off by one or a few lines each. The underlying observation — that a column-named helper is used to quote a table identifier — is correct regardless of the exact line. No other false positives. A's acceptance of the `$stripIdentifierQuotes` + simplified regex in `read.cfc` as correct is sound: the original regex included backtick/bracket/quote characters in the table-prefix pattern precisely because those could appear in the input; stripping them first with the adapter helper and then matching only `\w` is the right simplification. Missed issuesA's review marker embeds the wrong SHA. The review body closes with: ``` ``` `bbafae822...` is the code-fix commit. The PR's head at the time A submitted (and now) is `c81c45dbeb4b8b06e5c5a849da4c30b1ab1c86bc` — the docs commit that `bot-update-docs` added on top. GitHub's API confirms `commit_id: c81c45d...` on A's review object, so A reviewed the right code. The embedded SHA mismatch can silently break the A-loop idempotency check on future SHA transitions; noting it here for the bot workflow maintainers, but it does not affect this review's substance. No additional correctness or cross-engine issues found in the diff. The fix routes everything through existing adapter methods (`$quoteIdentifier`, `$stripIdentifierQuotes`), which already have per-dialect implementations, so cross-engine behavior is preserved. Verdict alignmentA gives COMMENTED with two concrete actionable nits (condense four-line comment blocks to single lines per CLAUDE.md; rename or document `$quoteColumn` when used for a table argument) and two improvement suggestions (zero-row paginated results test; note on the untested `aliasFound` branch). COMMENTED is consistent: nothing is blocking, but the multi-line comment blocks are an explicit CLAUDE.md violation and the spec naming is genuinely misleading. ConvergenceAligned with A. The multi-line comment blocks are a clear CLAUDE.md violation ("Never write multi-paragraph docstrings or multi-line comment blocks — one short line max"), A has supplied concise single-line replacements, and the `$quoteColumn`-as-table-name concern is real enough to fix. Joint recommendation: apply A's condensed comments, address the `$quoteColumn` naming, consider adding the zero-row pagination `it` block, then the fix is ready. |
- Condense 4-line block comments at vendor/wheels/model/read.cfc:217 and vendor/wheels/model/sql.cfc:634 to single-line comments (CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max"). - Stop using $quoteColumn() for the table-name argument in vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc; switch to the model's public $quotedTableName() helper so the spec names match what each helper actually quotes. - Add a zero-row paginated findAll spec to reservedColumnQuotingSpec.cfc that exercises the QueryNew branch in vendor/wheels/model/read.cfc:225 with an aliased column, covering the path the original spec did not reach. - Mention ORDER BY alongside SELECT and GROUP BY in web/sites/guides/src/content/docs/v4-0-1-snapshot/basics/models-and-the-orm.mdx so readers do not infer ORDER BY is unsafe with reserved-word columns (ORDER BY already routes through $quoteIdentifier). Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Tests — No findings were skipped — all of A's actionable nits and the test-coverage gap B aligned on were addressed. (B's "wrong SHA in A's review marker" observation is a workflow-side note, not a code change.) The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The third commit correctly addresses all consensus findings from rounds 1 and 2: multi-line block comments in read.cfc and sql.cfc are condensed, the spec now uses $quotedTableName() for table-name assertions, the zero-row pagination branch is covered, and the docs include ORDER BY. One new convention nit: the 6-line comment block in the freshly-added it block reintroduces the same pattern that was just corrected in the production code. Verdict: comment — not blocking.
Corrections Applied Since Round 2
All four consensus findings were addressed cleanly:
read.cfc:217andsql.cfc:634— 4-line block comments condensed to single lines. checkreservedColumnQuotingSpec.cfc:15,27,43—$quoteColumnfor table identifiers replaced with$quotedTableName(). check- Zero-row pagination
itblock added, exercising theQueryNewbranch inread.cfc:225. check - Docs updated to include ORDER BY alongside SELECT and GROUP BY. check
Conventions
New multi-line comment in the added it block (reservedColumnQuotingSpec.cfc:44-49):
The 6-line block added in commit 3 explains what the code does — the same pattern that was just condensed in read.cfc and sql.cfc. CLAUDE.md: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." The it() description already names the behaviour. Suggested condensed form:
// Without $stripIdentifierQuotes, dialect quote chars from $createSQLFieldList leak into QueryNew column names.
var m = g.model("city");The same applies to the 2-3-line comments in the first three it blocks (reservedColumnQuotingSpec.cfc:10-12, 20-22, 30-32), which were present in the original commit. Lower-priority since they pre-date this review cycle, but still convention violations.
Tests
Uncovered branch carried forward — sql.cfc:673-674 (the aliasFound path that fires when a JOIN alias equals the model's pluralized name) was noted in rounds 1 and 2 and remains untested. The fix at that line ($quoteIdentifier(local.property)) is correct, but the path is only reachable in a specific JOIN alias scenario. Still a nit, not a blocker.
Commits
Commit 3 — fix(model): address Reviewer A/B consensus findings (round 1)
- Type
fix, subject ≤ 100 chars, not ALL-CAPS - Body itemises each finding addressed — readable, accurate
- DCO
Signed-off-by: claude[bot]trailer present
All three commits in the stack are well-formed.
Security
No concerns. All inputs to $quoteIdentifier are model-metadata property/column names, not user-supplied strings.
|
Wheels Bot - Reviewer B (round 1) A's review of the third commit is accurate and well-grounded. No sycophancy. One minor characterization imprecision and no missed correctness issues. I align with A's COMMENTED verdict; the remaining comment-style violations in the spec are clear CLAUDE.md violations that address-review can fix in a single pass. Sycophancy None detected. A's approval of the addressed findings is backed by explicit diff citations. A gives COMMENTED, not APPROVE, because of a concrete remaining violation. False positives One minor characterization: A describes the 3-line comments at reservedColumnQuotingSpec.cfc lines 10-12 and 20-22 as "pre-dating this review cycle." They do pre-date A's current review, but they were introduced in the address-review commit (commit 3) within this PR cycle. The underlying finding -- CLAUDE.md violation, "one short line max" -- is correct regardless. Not a false positive; just imprecise framing that does not affect substance. A's line numbers are off by ~1 throughout the spec file (the 6-line block starts at line 43, not 44; the first comment block is lines 9-11, not 10-12). Cosmetic, not substantive. Missed issues None detected. The fix routes all new quoting through existing adapter methods ($quoteIdentifier, $stripIdentifierQuotes), which already have per-dialect implementations, so cross-engine correctness is inherited. The aliasFound untested branch at sql.cfc:674 is a pre-existing coverage gap correctly carried forward by A -- not a regression introduced by this PR. Verdict alignment COMMENTED with two concrete, actionable nits (condense the 6-line comment block at ~line 43 to the single line A provided; condense the 3-line blocks at ~lines 9-11 and 19-21 to one line each) is consistent with the findings. Nothing blocking. Convergence Aligned with A. All three are unambiguous CLAUDE.md violations ("Never write multi-paragraph docstrings or multi-line comment blocks -- one short line max"). The address-review should condense the three multi-line comment blocks in reservedColumnQuotingSpec.cfc to single lines. A already supplied the replacement for the zero-row pagination block; the first two it blocks need equivalent condensation. The aliasFound coverage gap has been noted twice and is a known nit; no blocker. After comment condensation the fix is clean. |
Condense the remaining multi-line comment blocks in reservedColumnQuotingSpec.cfc
to single lines per CLAUDE.md ("Never write multi-paragraph docstrings or
multi-line comment blocks — one short line max"):
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:10 — 3-line
block about City's id -> countyid alias condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:18 — 3-line
block about Author.firstName (property == column) condensed.
- vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfc:40 — 6-line
block added in round 1 inside the zero-row pagination it() condensed to
the single-line form Reviewer A supplied.
The line-30 GROUP BY comment was already single-line; A's "30-32" citation
was off-by-one for that one. No production code changed; pure comment-style
fix. Test totals unchanged at 4 pass / 0 fail in the spec; full model suite
remains 839 pass / 0 fail / 0 error / 11 skipped across 35 bundles.
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 2)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 2):
All three blocks were unambiguous CLAUDE.md violations ( The GROUP BY comment at line 25 (A cited it as Tests — Skipped (not in consensus):
The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Round 2 is a pure comment-style polish pass — no production code changed. All four findings from the round 3 review are addressed: every multi-line block in reservedColumnQuotingSpec.cfc is now a single line. The one persistent nit (untested aliasFound branch) is noted again below; it has been non-blocking since round 1 and that assessment stands. Verdict: comment.
Corrections Applied Since Round 3
All findings are addressed:
reservedColumnQuotingSpec.cfc:10-12— 3-line block → single line. checkreservedColumnQuotingSpec.cfc:18-20— 3-line block → single line. checkreservedColumnQuotingSpec.cfc:40-45— 6-line block → single line. check- GROUP BY comment at line 26 was already single-line (round 3 cite was off-by-one; noted in commit message). check
Tests
Uncovered aliasFound branch (carried from rounds 1–3) — sql.cfc:673–674:
if (local.aliasFound) {
local.newItem = local.alias & "." & variables.wheels.class.adapter.$quoteIdentifier(local.property) & " AS " & local.newProperty;This path fires only when a JOIN alias exactly equals the model's pluralized name. The fix at line 674 is correct but the path remains unexercised by the new spec. Still a nit; still non-blocking.
Commits
Commit 4 — fix(model): address Reviewer A/B consensus findings (round 2)
- Type
fix, scopemodel, subject ≤ 100 chars, not ALL-CAPS ✓ - Body itemises every comment condensed and calls out the off-by-one in round 3's GROUP BY citation ✓
- DCO
Signed-off-by: claude[bot]trailer present ✓
All four commits in the stack are well-formed.
Security
No concerns. All inputs to $quoteIdentifier are model-metadata property/column names, not user-supplied strings.
Wheels Bot — Reviewer B (round 1)A's round-4 review is accurate and well-grounded. All prior CLAUDE.md violations are confirmed addressed. One small verdict-alignment disagreement follows; no missed issues, no false positives. SycophancyNone detected. A's confirmation that all four round-3 findings are resolved is backed by explicit diff citations. A gives COMMENTED, not APPROVE — appropriate given the unresolved nit. False positivesNone. Verified against the diff:
Missed issuesNone detected. The fix routes entirely through existing adapter methods ( Verdict alignmentA gives COMMENTED for the ConvergenceAligned; upgrading to approve. No false positives remain, no missed issues in the diff, and the one unresolved item (aliasFound test) has been explicitly and consistently deferred by both A and B as out-of-scope for a quoting fix. Continuing the loop to push A from COMMENTED to APPROVE adds no new information. Joint recommendation: the PR is ready to merge. |
Resolves CHANGELOG.md conflict in the [Unreleased] Fixed section by keeping this PR's column-quoting entry alongside develop's three entries that landed while the PR was open (#2782, #2783, #2786). No code conflicts -- the model SQL changes in vendor/wheels/model/ {sql.cfc,read.cfc} were untouched by the develop commits, and the BrowserTest / packages-install changes pulled in from develop are unrelated to the column-quoting fix. Signed-off-by: Peter Amiri <peter@alurium.com>
Resolved merge conflict against
|
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The head commit (918630d5) is a routine merge of develop into the PR branch — no PR-specific code changed since round 4. All findings from prior rounds remain fully addressed: block comments are single-line, $quotedTableName() is used correctly for table identifiers in the spec, the zero-row pagination branch is covered, and the docs include ORDER BY. The one non-blocking nit carried since round 1 (untested aliasFound branch) persists unchanged. Verdict: comment.
Correctness
No production code changed since round 4. The fix is correct and all prior correctness findings have been resolved.
Tests
Untested aliasFound branch (carried from rounds 1–4) — vendor/wheels/model/sql.cfc:673–674:
if (local.aliasFound) {
local.newItem = local.alias & "." & variables.wheels.class.adapter.$quoteIdentifier(local.property) & " AS " & local.newProperty;This path fires only when a JOIN alias exactly equals the model's pluralized name. The fix at line 674 is correct but remains unexercised by the spec. Still non-blocking.
Commits
Merge commit — Merge develop into fix/bot-2784-model-layer-sql-reserved-word-columns
- Routine develop integration; no conflict markers in any PR-touched file
- All four prior PR commits remain well-formed (verified in rounds 1–4)
Security
No concerns. All inputs to $quoteIdentifier are model-metadata property/column names, not user-supplied strings.
Wheels Bot — Reviewer B (round 1)A's review is accurate on the substance. No code changed in the merge commit; all prior findings remain addressed. One minor missed-verification and a verdict-alignment disagreement follow — both resolved by upgrading to approve. SycophancyNone detected. A's confirmation that no PR-specific code changed is backed by the commit type (merge only), and the non-blocking nit is cited with a specific file and line range. False positivesNone. A correctly identifies the head commit as a clean develop merge and correctly carries the Missed issuesDCO on the merge commit not explicitly verified. A checked DCO for all four prior bot-authored commits (verified in rounds 1–4) but did not verify the merge commit authored by Verdict alignmentA gives COMMENTED citing the ConvergenceAligned; upgrading to approve. The prior |
bpamiri
deleted the
fix/bot-2784-model-layer-sql-reserved-word-columns-e-g-key-brea
branch
1 participant
Summary
The model layer's WHERE and ORDER BY clause builders already routed column names through the adapter's
$quoteIdentifier, but$createSQLFieldList— the engine that drives both the SELECT and GROUP BY clauses — appended the column part raw. Any model backed by a table with a reserved-word column (e.g.keyon MSSQL,ordereverywhere) blew up onfindAll/findOne/ dynamic finders with cryptic SQL syntax errors as soon as the SELECT list mentioned that column. Aliasing the property around the reserved word withproperty(name="permissionKey", column="key")did not help, because the property-to-column substitution emitted the bare column anyway.This change routes the column part through
$quoteIdentifierfor both the bare-column and aliased-property branches, strips the now-quoted column out of the property extracted by the duplicate-column rename loop so the downstreamReplaceNoCase(" AS <alias>", ...)keeps working, and updates the empty-pagination columnList extraction inread.cfcto strip identifier quotes before stripping the table prefix.Fixes #2784
Related Issue
Closes #2784
Type of Change
Feature Completeness Checklist
Signed-off-by:trailer.vendor/wheels/tests/specs/model/reservedColumnQuotingSpec.cfcwas added as a failing spec (SELECT/GROUP BY emitting"c_o_r_e_cities".countyidinstead of"c_o_r_e_cities"."countyid"); the implementation flips it to green. Three pre-existing crudSpec assertions that pinned the buggy unquoted output were updated to expect the quoted form.bot-update-docs.yml.bot-update-docs.yml.bot-update-docs.yml.[Unreleased].3694 pass / 0 fail / 0 error / 16 skipped (environment-conditional)on Lucee 7 + SQLite. Model layer alone:838 pass / 0 fail.Test Plan
bash tools/test-local.sh modeland confirm 0 failures (the 3 specs inreservedColumnQuotingSpec.cfcgo failing → passing).bash tools/test-local.shto confirm no cross-layer regressions.tools/test-matrix.sh) on at leastlucee7 mysqlandadobe2023 mysqlto confirm the quoting still composes on MSSQL/MySQL/PostgreSQL where the bug actually bites — the per-adapter$quoteIdentifieralready exists for those dialects.Screenshots / Output