Skip to content

fix(model): Oracle insertAll on Lucee and explicit identity keys on SQL Server - #3655

Open
bpamiri wants to merge 8 commits into
developfrom
claude/vigilant-pasteur-l8sjld
Open

bpamiri wants to merge 8 commits into
developfrom
claude/vigilant-pasteur-l8sjld

Conversation

@bpamiri

@bpamiri bpamiri commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR works through the repo's open issues. It fixes the two open framework bugs, fixes a third bug that one of the new specs exposed, closes out the CLI/MCP roadmap item as far as this repo can, and closes three issues that were already fixed on develop.

Fixed here

  • Oracle + Lucee: insertAll appends RETURNING ROWID to the FROM dual bulk-insert shape (ORA-03048) #3653: insertAll() fails on Oracle under Lucee (ORA-03048 … FROM dual RETURNING). A cfquery result attribute makes Lucee request generated keys from the driver. The Oracle driver implements that by appending RETURNING ROWID INTO ? to every INSERT, and Oracle rejects that clause after the INSERT … SELECT … FROM dual UNION ALL bulk shape. I confirmed this with a probe on Lucee 7 + ojdbc11: the same statement succeeds without result=, and a leading comment doesn't help. Nothing on the bulk paths reads the result or a key, so insertAll() and upsertAll() now pass $captureResult = false, and $performQuery() drops the attribute. Every other query is unchanged. I rejected a PL/SQL BEGIN … END; wrapper: it halves Oracle's bind capacity (1,000 × 40 binds fails with ORA-16951, while the plain statement passes) and it was slower.
  • Model create() with an explicit primary key fails on SQL Server (IDENTITY_INSERT) #3647: create(id = 41, …) fails on SQL Server (IDENTITY_INSERT is set to OFF). When an INSERT's column list carries the primary key, MicrosoftSQLServerModel.$querySetup() now wraps it via $identityInsertSQL(). The wrapper is one batch: a sys.identity_columns-guarded SET IDENTITY_INSERT … ON, the INSERT, then the matching OFF. A natural or UUID key stays a plain INSERT, and every other adapter is untouched. The wrapper deliberately has no TRY/CATCH. I tried CATCH + THROW, and Lucee dropped the re-raised error, so a duplicate key came back as a successful create(). With inlined values the setting also outlives the batch on the pooled connection, so the trailing OFF has to run even when the INSERT fails. A constraint violation only ends its own statement, so it does.
  • Found along the way: a failed transaction = "none" model call disabled transactions for the rest of the request. invokeWithTransaction() sets the per-request "transaction open" marker in none/false mode too, and a throw skipped the reset. Every later save()/create() in that request then ran with no transaction, and transaction = "rollback" stopped rolling back. The core runner uses transactionMode = "none", so the new duplicate-key spec made OuterTransactionSignalSpec fail later in the same run. The commit/rollback branch already resets the marker on errors (compat-matrix: engine-job failures are invisible (continue-on-error) — harden after burning down pre-existing leg debt #3302); the none/false branch now does the same.
  • roadmap: high-impact CLI/MCP tooling-honesty gaps — failure exit codes (incl. upgrade check) + MCP tool input schemas #2963 (MCP tooling honesty): the runtime half has landed. I ran a stdio tools/list against this module on the pinned LuCLI 0.6.1. All 12 ArgSpec-backed tools advertise their declared mcpToolSpecs() schemas, and no $-helpers leak. The one leftover was dbmigrate, the migrate alias. It was still advertised as a duplicate tool with an empty schema, while d/g were already hidden. It's now hidden too, and the stale mcpToolSpecs() comment is corrected.

Already fixed on develop, closed here as housekeeping

Left open

Related Issue

Closes #3653
Closes #3647
Closes #3649
Closes #3627
Closes #3427
Refs #2963

Type of Change

  • Bug fix
  • New feature
  • Enhancement to existing feature
  • Documentation update
  • Refactoring

Feature Completeness Checklist

  • DCO sign-off: every commit carries Signed-off-by:
  • Tests: see below; each new spec was mutation-checked against a reverted fix
  • Framework Docs: n/a (bug fixes; no user-facing API change beyond an explicit key now working on SQL Server)
  • AI Reference Docs: .ai/wheels/cross-engine-compatibility.md. The Oracle bulk-insert section still recommended the INSERT ALL shape compat-matrix: engine-job failures are invisible (continue-on-error) — harden after burning down pre-existing leg debt #3302 replaced; it's rewritten, and a SQL Server section records the two probed batch facts above
  • CLAUDE.md: n/a (no convention changes)
  • Changelog fragment: 3653-oracle-lucee-bulk-insert, 3647-sqlserver-explicit-identity-insert, transaction-none-marker-reset, mcp-hide-dbmigrate-alias
  • Test runner passes: see Test Plan

Test Plan

New specs:

  • database/BulkResultCaptureSpec: a spy adapter checks that insertAll()/upsertAll() opt out of the result, and $performQuery() is checked with and without the flag. This runs on every leg, because the Oracle legs are soft-fail and weekly.
  • model/explicitIdentityInsertSpec: live on every database. An explicit key persists, the next generated key still works on the same connection, and on SQL Server a duplicate explicit key raises and leaves the connection usable.
  • database/MicrosoftSQLServerUnitSpec: pins the wrapper and the $querySetup() wiring on every leg, including the BoxLang SCOPE_IDENTITY() interaction.
  • model/TransactionMarkerResetSpec: a throwing transaction = "none" call clears the marker it set, and a nested call leaves an outer owner's marker alone.
  • seederSpec: drops the hand-rolled IDENTITY_INSERT workaround from test(seed): make the seeder FK fixtures identity-aware on SQL Server #3648 and goes back to plain create() calls.

Mutation checks (fix reverted → spec fails):

Full core suite, run locally in the repo's Docker images:

Engine Database Pass Fail Error
Lucee 7 SQLite 5729 0 0
Lucee 7 MySQL 5734 0 0
Lucee 7 PostgreSQL 5725 0 0
Lucee 7 SQL Server 5730 0 0
Lucee 7 Oracle 5721 0 0
Lucee 7 H2 5721 0 0
Adobe 2023 SQLite 5729 0 0
Adobe 2023 MySQL 5734 0 0
Adobe 2023 SQL Server 5730 0 0
Adobe 2023 Oracle ¹ 5721 0 0

¹ Run with a 2 GB heap (a local CommandBox override). At this container's default 768 MB heap, Adobe + Oracle runs out of memory inside ojdbc's row-prefetch buffers, and develop does exactly the same; on develop at 2 GB it gives 5705 pass, 0 fail. So the OOM isn't related to this PR.

These numbers are from the branch before I rebased it onto #3654, which only touches docs and prompts. After the rebase I re-ran Lucee 7 + SQLite: 5727 pass, 0 fail, 0 error.

CLI suite (tools/test-cli-local.sh on LuCLI 0.6.1): 1386 pass. The 4 DbCommandSpec failures expect no running server, and they fail identically on develop's module in this environment (1385 pass, 4 fail).

BoxLang, full core suite on the current head (4dc944f):

Engine Database Pass Fail Error
BoxLang SQLite 5727 0 0
BoxLang SQL Server 5728 0 0
BoxLang MySQL 5732 0 0
BoxLang PostgreSQL 5723 0 0

Compatibility matrix. The first dispatch (run 35817080185, on 795cf2f) was green on every Adobe 2023, Adobe 2025 and Lucee 6 leg, and on every Lucee 7 leg except SQLite. Three things failed:

  • Lucee 7 + SQLite errors with cannot load class org.sqlite.JDBC. It fails the same way on the develop-based run 35792180182, so it isn't from this PR.
  • RustCFML failed on TransactionMarkerResetSpec. 9eaf3fe fixed that, and PR CI's RustCFML job is now green.
  • Every BoxLang leg failed on explicitIdentityInsertSpec:
    • BoxLang escapes an inlined string literal's quotes a second time (parameterize = false), and the spec's two inlined inserts carried a name string. That gave syntax errors on MySQL, PostgreSQL and CockroachDB, and a null-pointer error on SQLite.
    • On SQL Server the driver exception also overflowed BoxLang's JSON serializer, so the whole leg came back as a StackOverflowError page.
    • 4dc944f drops the string values from those inserts. Only the key matters to these cases.
    • Before the change, I reproduced the SQLite and SQL Server failures locally with the old spec.
    • The double escaping itself is older than this PR. It lives in Base.$executeQuery's inlined-value branch, and crudSpec already skips its parameterize = false cases on BoxLang. I've left it for a follow-up rather than widening this PR.

The re-dispatch on 4dc944f (run 35820444015) is green on 34 of its 35 jobs, including every BoxLang, Adobe 2025 and Oracle leg. The one red job is Lucee 7 + SQLite, which already failed before this PR (above).

Not run locally: Adobe 2025, CockroachDB and Lucee 6. The matrix covers them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU

…3653)

insertAll() failed on every Lucee + Oracle leg with ORA-03048 ("SQL reserved
word 'ROWID' is not syntactically valid following '... FROM dual RETURNING'").
A cfquery `result` attribute makes Lucee request generated keys
(Statement.RETURN_GENERATED_KEYS), and ojdbc implements that by appending
`RETURNING ROWID INTO ?` to every INSERT. Oracle rejects that after the
`INSERT ... SELECT ... FROM dual UNION ALL` bulk shape. Probed on Lucee 7 +
ojdbc11: the same statement succeeds when no `result` attribute is requested.

Nothing on the bulk paths reads the result struct or a generated key, so
insertAll() and upsertAll() now pass `$captureResult = false` and
$performQuery() omits the attribute; $executeQuery() returns an empty result
for such calls instead of running the identity lookup. Every other query keeps
its result metadata.

Wrapping the statement in a PL/SQL block was rejected: it halves Oracle's bind
capacity (40,000 binds fail with ORA-16951 where plain SQL passes) and was
several times slower.

BulkResultCaptureSpec pins the contract on every leg, since the Oracle legs are
soft-fail and weekly: a spy adapter checks that both bulk paths opt out, and
$performQuery is checked with and without the flag. Reverting the fix brings
back the four bulkOperationsSpec errors on Lucee 7 + Oracle and fails both new
contract specs.

Verified on Lucee 7: full core suite green on SQLite, MySQL, PostgreSQL, SQL
Server, Oracle and H2. Before the fix, the Oracle model specs alone were 1009
pass + 4 errors.

Closes #3653

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
…ll throws

invokeWithTransaction() sets request.wheels.transactions[connectionArgs] before
it dispatches, for the "none"/"false" modes too, so nested model calls skip
their own transaction. The commit/rollback branch resets that marker when the
invoked method throws (#3302), but the none/false branch did not. One failing
save(transaction = "none") therefore left the marker set for the rest of the
request: every later save()/create()/update() took the "alreadyopen" path and
ran with no transaction, and transaction = "rollback" stopped rolling back.

The core test runner runs with transactionMode = "none", which is how this
surfaced: a spec whose create() hit a duplicate key on SQL Server made
OuterTransactionSignalSpec's rollback control fail later in the same request
(8 tags expected, 9 found). A marker trace confirmed the flag was stuck at true
after a single failed create(), with or without an outer transaction.

The none/false branch now resets the marker it set when the method throws. A
nested ("alreadyopen") call does not own the marker and leaves it alone; the
outer owner clears its own.

TransactionMarkerResetSpec gains both cases. Reverting the fix fails the new
"transaction='none' call throws" case on Lucee 7 + H2.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
…ver (#3647)

model("RefParent").create(id = 41, ...) failed on SQL Server with "Cannot
insert explicit value for identity column ... when IDENTITY_INSERT is set to
OFF", while every other supported database accepts an explicit value. The
migrator already had the ON/OFF pair (addRecordPrefix/addRecordSuffix), but
the model insert path never used it.

The SQL Server adapter's $querySetup() now wraps an INSERT whose column list
carries the primary key (the same "the caller supplied the key" test
$identitySelect already uses) via the new $identityInsertSQL():

  DECLARE @wheelsIdentityInsert bit = CASE WHEN EXISTS (SELECT 1 FROM
    sys.identity_columns WHERE object_id = OBJECT_ID(N'[t]') AND name IN (N'id'))
    THEN 1 ELSE 0 END; IF @wheelsIdentityInsert = 1 SET IDENTITY_INSERT [t] ON;
  INSERT INTO [t] (...) VALUES (...)
  ; IF @wheelsIdentityInsert = 1 SET IDENTITY_INSERT [t] OFF;

- One batch, so the ON, the INSERT and the OFF share a connection whether or
  not the caller opened a transaction.
- Catalog-guarded, so a natural or UUID key stays a plain INSERT (SET
  IDENTITY_INSERT on a table without an identity column is an error).
- No TRY/CATCH. Probed on Lucee 7 + SQL Server: with inlined values the setting
  outlives the batch, so the OFF must run even when the INSERT fails. A
  constraint violation only ends its own statement, so the trailing OFF still
  runs and the next insert on that connection works. A CATCH + THROW wrapper
  lost the error instead: Lucee drops an error raised after a batch's first
  result, so a duplicate key came back as a successful create().

Every other adapter is untouched. Wrapping happens before the BoxLang
SCOPE_IDENTITY() append, which correctly skips a wrapped statement because the
caller already has the key.

Specs: model/explicitIdentityInsertSpec runs live on every database (explicit
key persists; the next generated key still works on the same connection; a
duplicate explicit key raises and the connection stays usable).
database/MicrosoftSQLServerUnitSpec pins the wrapper and the $querySetup wiring
on every leg. seederSpec drops the hand-rolled IDENTITY_INSERT workaround from
#3648 and goes back to plain create() calls.

Mutation-checked on Lucee 7 + SQL Server: reverting the adapter change gives
the reported error three times, and dropping only the trailing OFF fails the
two same-connection specs ("Explicit value must be specified for identity
column").

Closes #3647

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
With the pinned LuCLI runtime (0.6.1), a stdio `tools/list` against this
module returns 19 tools. The 12 ArgSpec-backed ones carry their declared
mcpToolSpecs() schemas (migrate's 10 properties match migrateArgSpec()
exactly), and no $-helpers leak. So the runtime half of #2963 has landed.

The one duplicate is `dbmigrate`, the historical alias for migrate. `d` and `g`
were already hidden for the same reason. Advertised, the alias is a second copy
of migrate with an empty inputSchema, since it has no mcpToolSpecs() entry.
Hide it with the other aliases, and correct the mcpToolSpecs() comment that
still listed generate, migrate and create as unmigrated. Those are registered;
db, deploy, packages and reload are the commands that still parse their own
argv, and info and validate take no arguments.

McpHiddenToolsSpec gains an alias assertion. CLI suite (LuCLI 0.6.1): 1386
pass. The 4 DbCommandSpec failures expect no running server and fail
identically on develop's module (1385 pass, 4 fail) in this environment.

Refs #2963

Signed-off-by: Claude <noreply@anthropic.com>
The "Oracle — Multi-Row INSERT and RETURNING Incompatibility" section still
described `INSERT ALL ... SELECT 1 FROM dual` as the bulk shape and told
readers to use it. #3302 replaced that form because its single driving row gave
every record the same generated key. It also never mentioned what triggers the
RETURNING rewrite on Lucee: a cfquery `result` attribute.

Rewrite it around the current mechanism (INSERT ... SELECT ... UNION ALL, run
with $captureResult = false; #3653), including why a PL/SQL wrapper was
rejected. Add a SQL Server section for #3647 recording two probed engine facts
that shaped $identityInsertSQL(): Lucee drops an error raised after a batch's
first result (so CATCH + THROW reports a failed insert as success), and a SET
in a statement with inlined values outlives the batch on the pooled connection.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
On Adobe CF a database error's `message` is the generic "Error Executing
Database Query."; the driver's own text ("Violation of PRIMARY KEY
constraint ...") is in `detail`. The duplicate-key case in
explicitIdentityInsertSpec only read `message`, so it failed on Adobe 2023 +
SQL Server even though the error did surface. Read both.

Adobe 2023 + SQL Server: 5730 pass, 0 fail, 0 error with this change (5729 + 1
fail before it).

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
@github-actions

github-actions Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

     31 files   12 122 suites   27m 36s ⏱️
164 456 tests 163 503 ✅ 953 💤 0 ❌
166 602 runs  165 649 ✅ 953 💤 0 ❌

Results for commit 4dc944f.

♻️ This comment has been updated with latest results.

The two TransactionMarkerResetSpec cases added with the transaction="none"
marker fix forced the invoked method to throw by calling findByKey() without its
required key. RustCFML does not enforce required arguments through cfinvoke, so
the call returned instead of throwing, and both cases failed on the RustCFML
(Linux) check: "findByKey() without a key should have thrown."

Use findAll(select = "wheelsNoSuchColumn") instead. It throws
Wheels.ColumnNotFound from Wheels' own Throw() inside the invoked method, after
the marker is set and before any query runs. raisedErrorsSpec already relies on
the same behaviour on every engine.

Reproduced both failures locally on RustCFML v0.637.0. After the change:
RustCFML model specs 1017 pass, 0 fail; tools/rustcfml/run-suite.sh 5726 pass
and exit 0 (only the baseline's migrationSpec addIndex entry remains). With the
marker fix reverted, the "transaction='none' call throws" case still fails on
RustCFML. Lucee 7 + H2 and Adobe 2023 + MySQL model specs: 0 fail.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU
Two explicitIdentityInsertSpec cases run their explicit-key insert with parameterize=false, so SQL
Server executes it as a plain batch where IDENTITY_INSERT outlives the statement. Both inserts
also carried a name string. BoxLang escapes the quotes of an inlined string literal a second time
(the SQL reached the driver as VALUES (942, ''Explicit key 942'')), which failed every BoxLang leg
of the compatibility matrix:

- mysql, postgres, cockroachdb: a syntax error at "Explicit"
- sqlite: a null-pointer error from the driver
- sqlserver: the driver's exception object overflowed BoxLang's JSON serializer, so the leg's
  whole result came back as a StackOverflowError page

The name was incidental; only the key matters to what these cases check. The inlined inserts now
set just the key (the name column is nullable), which keeps the direct-batch coverage on every
engine. The double escaping is in Base.$executeQuery's parameterize=false value branch and
predates this change; crudSpec already skips its parameterize=false cases on BoxLang for it.

Verified locally:

- BoxLang, model specs: the old spec reproduces both failures (sqlite 1 error, sqlserver the
  StackOverflowError page); with this change sqlite and sqlserver pass all three cases.
- BoxLang, full core suite: sqlite 5727 pass, sqlserver 5728 pass, 0 fail and 0 error on both.
- Lucee 7 (sqlserver, h2, sqlite) and Adobe 2023 (sqlserver): model specs green.
- Dropping the wrapper's trailing IDENTITY_INSERT OFF still fails both same-connection cases on
  Lucee 7 + SQL Server.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiiotnF2YZPUFbk7U7pFXU

bpamiri commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

Compat-matrix status for this PR. Run 35817080185 was on 795cf2f.

Fixed in this PR

  • BoxLang legs: fixed by 4dc944f. They failed on explicitIdentityInsertSpec, whose parameterize = false inserts carried a string that BoxLang double-escapes.
  • rustcfml: fixed by 9eaf3fe.

Not this PR's: lucee7 + sqlite

  • Symptom: the leg never runs a spec. All three attempts return HTTP 500 with cannot load class ... [org.sqlite.JDBC].
  • Already on develop: the develop-based run 35792180182 fails with the identical error.
  • Cause: tools/docker/lucee7/Dockerfile installs sqlite-jdbc.jar into WEB-INF/lucee-server/context/lib. Lucee 6 loads drivers from there, but Lucee 7 (7.0.1+100) doesn't.
  • Local check: after copying the same jar into /usr/local/lib/serverHome/WEB-INF/lib, the full core suite ran on Lucee 7 + SQLite with 0 failures and 0 errors.

No fix exists yet. The proposed patch is harness-only, so I'm not adding it to this PR:

- && mkdir -p /usr/local/lib/serverHome/WEB-INF/lucee-server/context/lib \
- && wget -q -O /usr/local/lib/serverHome/WEB-INF/lucee-server/context/lib/sqlite-jdbc.jar \
+ && mkdir -p /usr/local/lib/serverHome/WEB-INF/lib \
+ && wget -q -O /usr/local/lib/serverHome/WEB-INF/lib/sqlite-jdbc.jar \
       https://repo1.maven.org/maven2/org/xerial/sqlite-jdbc/3.50.3.0/sqlite-jdbc-3.50.3.0.jar

The matrix is re-dispatched on 4dc944f (run 35820444015), which re-runs this leg once.


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment