Skip to content
Merged
22 changes: 18 additions & 4 deletions .ai/wheels/cross-engine-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -532,13 +532,27 @@ if (isMySQLFamily) {

CockroachDB is a full (non-soft-fail) leg of the compat matrix — each engine × cockroachdb combination runs as its own parallel job in `.github/workflows/compat-matrix.yml`. The only remaining soft-fail database is Oracle (`SOFT_FAIL_DBS="oracle"` in the same workflow, tracked in #2663).

### Oracle — Multi-Row INSERT and RETURNING Incompatibility
### Oracle — Bulk INSERT, RETURNING and Generated Keys

Oracle 23 rejects `INSERT INTO t (cols) VALUES (?,?), (?,?), ...` (the SQL-standard table value constructor) when the JDBC driver also requests `RETURN_GENERATED_KEYS`. The Oracle JDBC driver translates `RETURN_GENERATED_KEYS` into a `RETURNING ROWID INTO` clause, and Oracle 23 does not permit `RETURNING` combined with multi-row VALUES.
The Oracle JDBC driver implements `Statement.RETURN_GENERATED_KEYS` by appending `RETURNING ROWID INTO ?` to every INSERT it is handed, and Oracle rejects that clause after two bulk shapes:

`OracleModel` overrides `$bulkInsertSQL()` to emit `INSERT ALL INTO t (cols) VALUES (...) INTO t (cols) VALUES (...) SELECT 1 FROM dual` — Oracle's idiomatic multi-row form, which avoids both the table value constructor and the RETURNING expansion. This is transparent to framework users; `insertAll()` works the same on Oracle as on other databases.
- a multi-row `VALUES (?,?), (?,?)` table value constructor — `returning clause is not allowed with INSERT and Table Value Constructor` on Oracle 23 (#2745);
- `INSERT INTO t (cols) SELECT ... FROM dual UNION ALL ...` — `ORA-03048: SQL reserved word 'ROWID' is not syntactically valid following '... FROM dual RETURNING'` (#3653).

If you write code that generates raw bulk-insert SQL for Oracle (or adds a new adapter), use `INSERT ALL ... SELECT 1 FROM dual` rather than multi-row VALUES. The canonical implementation is `vendor/wheels/databaseAdapters/Oracle/OracleModel.cfc::$bulkInsertSQL`.
Lucee requests generated keys for any `cfquery` that carries a `result` attribute. Probed on Lucee 7 + ojdbc11, the same `INSERT ... SELECT` succeeds once `result` is dropped, and a leading `/* comment */` does not help because the driver skips comments when it classifies the statement.

How the framework handles it:

- `OracleModel::$bulkInsertSQL` emits `INSERT INTO t (cols) SELECT ... FROM dual UNION ALL SELECT ... FROM dual`: one driving row per record, so identity defaults are evaluated per row. The older `INSERT ALL ... SELECT 1 FROM dual` form had a single driving row and handed every record the same generated key (#3302), so don't reintroduce it.
- `insertAll()` and `upsertAll()` run their statements with `$performQuery(..., $captureResult = false)`, which omits the `result` attribute, so no RETURNING clause is appended. Anything else that runs a multi-row INSERT through `$performQuery` and does not read the result or a key should opt out the same way.
- Don't dodge the rewrite with a PL/SQL block (`BEGIN INSERT ...; END;`). It works, but it halves the bind capacity: 1,000 rows × 40 columns fails with `ORA-16951` where the plain statement passes. It was also several times slower in the same probe.

### SQL Server — Explicit Identity Values and Multi-Statement Batches

`create()` with an explicit primary key needs `SET IDENTITY_INSERT <table> ON` when that key is an IDENTITY column (#3647). `MicrosoftSQLServerModel::$identityInsertSQL` sends the ON, the INSERT and the OFF as one batch, guarded by a `sys.identity_columns` lookup. Two engine facts shaped it, both probed on Lucee 7 + SQL Server:

- **Lucee drops an error raised after a batch's first result.** Wrapping the INSERT in `BEGIN TRY ... END TRY BEGIN CATCH ... THROW; END CATCH` made a duplicate-key insert report success: the failed statement's result came back first, and Lucee never surfaced the re-raised error that followed it. Don't use CATCH + THROW (or `RAISERROR` after other output) to report failures from a multi-statement batch.
- **A `SET` in a statement with inlined values outlives the batch** and stays on the pooled connection. With `parameterize = false`, a leftover `IDENTITY_INSERT ON` made the connection's next plain insert fail ("Explicit value must be specified for identity column"). So the OFF must run even when the INSERT fails. A constraint violation only ends its own statement, so a trailing OFF in the same batch still runs.

### Oracle — DDL Auto-Commit and Transaction Wrapper

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `create()` with an explicit primary key now works on SQL Server. `model("Post").create(id = 41, ...)` used to fail with `Cannot insert explicit value for identity column ... when IDENTITY_INSERT is set to OFF`, while every other supported database accepted it. When an INSERT's column list carries the primary key, the SQL Server adapter now sends it in one batch with `SET IDENTITY_INSERT <table> ON` before and `OFF` after, both guarded by a `sys.identity_columns` lookup so a natural or UUID key stays a plain INSERT. The OFF runs even when the INSERT hits a constraint violation, so a failed insert cannot leave the setting on for the pooled connection (#3647)
1 change: 1 addition & 0 deletions changelog.d/3653-oracle-lucee-bulk-insert.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `insertAll()` no longer fails on Oracle under Lucee with `ORA-03048: SQL reserved word 'ROWID' is not syntactically valid following '... FROM dual RETURNING'`. A cfquery `result` attribute makes Lucee request generated keys from the driver, and the Oracle driver implements that by appending `RETURNING ROWID INTO ?` to every INSERT, which Oracle rejects after the `INSERT ... SELECT ... FROM dual UNION ALL` bulk shape. Nothing on the bulk paths reads the result or a key, so `insertAll()` and `upsertAll()` now run their statements without one (`$performQuery(..., $captureResult = false)`); the Lucee + Oracle core suite goes from four `bulkOperationsSpec` errors to green (#3653)
1 change: 1 addition & 0 deletions changelog.d/mcp-hide-dbmigrate-alias.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The `wheels mcp wheels` tool list no longer advertises `dbmigrate`. It is the historical alias for `migrate` (like `d` for destroy and `g` for generate, which were already hidden), and it has no `mcpToolSpecs()` entry, so MCP clients saw it as a no-argument twin of `migrate`. The stdio `tools/list` on the pinned LuCLI runtime (0.6.1) now carries one entry per command, with the declared input schema on every ArgSpec-backed tool (#2963)
1 change: 1 addition & 0 deletions changelog.d/transaction-none-marker-reset.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- A model call made with `transaction = "none"` (or `false`) that throws no longer disables transaction handling for the rest of the request. `invokeWithTransaction()` sets the request's open-transaction marker for those modes too, and a throw from the invoked method skipped the reset, so every later `save()`/`create()`/`update()` in that request took the "already open" path and ran with no transaction, and `transaction = "rollback"` stopped rolling back. The commit/rollback branch already reset the marker on errors (#3302); the `none`/`false` branch now does the same
9 changes: 6 additions & 3 deletions cli/lucli/Module.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ component extends="modules.BaseModule" {
"map", // deprecated forwarder for the same thing (snapshot 2499)
"d", // alias for destroy
"g", // alias for generate
"dbmigrate", // alias for migrate — a duplicate tool with no inputSchema otherwise
"new", // scaffolds a whole new Wheels project
"console", // interactive CFML REPL — not usable over stdio
"start", // dev server lifecycle (stateful)
Expand Down Expand Up @@ -255,9 +256,11 @@ component extends="modules.BaseModule" {
* builder the command's parse helper uses, so the CLI parse surface and
* the MCP advertisement cannot drift.
*
* Commands still on hand-rolled token parsing (generate, migrate, db,
* deploy, info, reload, validate, create — tracked by #2861)
* gain entries here as they migrate to ArgSpec.
* Verified over stdio on the pinned LuCLI runtime (0.6.1): every entry
* below is advertised as its tool's inputSchema. `info` and `validate`
* take no arguments, so their empty schema is accurate. The commands that
* still parse their own argv (db, deploy, packages, reload) advertise an
* empty schema until they move to ArgSpec and gain an entry here.
*/
public struct function mcpToolSpecs() {
return {
Expand Down
13 changes: 13 additions & 0 deletions cli/lucli/tests/specs/commands/McpHiddenToolsSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ component extends="wheels.wheelstest.system.BaseSpec" {
expect(body).toInclude("""mcp""");
});

it("hides the command aliases so each command is advertised once", () => {
// `d`, `g` and `dbmigrate` forward to destroy / generate / migrate.
// Advertised, an alias is a second copy of the tool with an empty
// inputSchema (it has no mcpToolSpecs() entry), so an MCP client sees
// `dbmigrate` as a no-argument twin of `migrate`.
var startIdx = reFindNoCase("(?m)^[ \t]*public\s+array\s+function\s+mcpHiddenTools\s*\(", variables.moduleSource);
expect(startIdx).toBeGT(0);
var body = mid(variables.moduleSource, startIdx, 2500);
expect(body).toInclude("""d""");
expect(body).toInclude("""g""");
expect(body).toInclude("""dbmigrate""");
});

it("returns an array (the LuCLI mcpHiddenTools() contract)", () => {
// LuCLI calls mcpHiddenTools() and expects an array of
// string names. Source-level: the return type is `array`
Expand Down
31 changes: 25 additions & 6 deletions vendor/wheels/databaseAdapters/Base.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ component output=false extends="wheels.Global"{
wheels.rv.query = local[args.debugName];
}

// No `result` attribute was requested ($performQuery's $captureResult=false,
// the bulk paths): there is no result metadata to return and no generated
// key to look up.
if (!structKeyExists(wheels, "result")) {
wheels.rv.result = {};
return wheels.rv;
}

// Manual identity retrieval for Lucee / ACF
// Pass the query result (if any) as returningIdentity — needed by adapters
// that use RETURNING clauses (e.g. CockroachDB) to retrieve generated keys.
Expand Down Expand Up @@ -846,6 +854,10 @@ component output=false extends="wheels.Global"{

/**
* Internal function.
*
* @$captureResult Pass `false` when the caller reads neither the cfquery result
* struct nor a generated key (the bulk paths). The `result` attribute is then
* omitted, which is what stops Lucee asking the driver for generated keys.
*/
public struct function $performQuery(
required array sql,
Expand All @@ -854,7 +866,8 @@ component output=false extends="wheels.Global"{
numeric offset = 0,
string dataSource = variables.dataSource,
string $primaryKey = "",
string $debugName = "query"
string $debugName = "query",
boolean $captureResult = true
) {
// Multi-tenant datasource override: if a tenant is active and this model
// is not shared, route the query to the tenant's datasource.
Expand All @@ -873,7 +886,15 @@ component output=false extends="wheels.Global"{
local.queryAttributes.dataSource = arguments.dataSource;
local.queryAttributes.username = variables.username;
local.queryAttributes.password = variables.password;
local.queryAttributes.result = "local.wheels.result";
// A `result` attribute is not free: Lucee requests generated keys
// (Statement.RETURN_GENERATED_KEYS) for any cfquery that has one, and the
// Oracle driver implements that by appending `RETURNING ROWID INTO ?` to every
// INSERT. Oracle rejects that after `INSERT ... SELECT`, so insertAll() failed
// with ORA-03048 on Lucee (#3653). Callers that read neither the result nor a
// key opt out with $captureResult=false.
if (arguments.$captureResult) {
local.queryAttributes.result = "local.wheels.result";
}
local.queryAttributes.name = "local." & arguments.$debugName;
if (StructKeyExists(local.queryAttributes, "username") && !Len(local.queryAttributes.username)) {
StructDelete(local.queryAttributes, "username");
Expand All @@ -897,7 +918,7 @@ component output=false extends="wheels.Global"{
// Copy only the non-excluded keys by reference — Duplicate(arguments) would
// deep-clone the entire SQL fragment array (including param structs) per query.
for (local.key in arguments) {
if (!ListFindNoCase("sql,parameterize,$debugName,limit,offset,$primaryKey", local.key)) {
if (!ListFindNoCase("sql,parameterize,$debugName,limit,offset,$primaryKey,$captureResult", local.key)) {
local.queryAttributes[local.key] = arguments[local.key];
}
}
Expand All @@ -917,9 +938,7 @@ component output=false extends="wheels.Global"{
* Generates a multi-row INSERT statement as an array compatible with `$querySetup()`.
* Default shape is `INSERT INTO ... VALUES (?,?), (?,?), ...` (SQL standard table value
* constructor) — used by every adapter except Oracle, which overrides this method to
* emit `INSERT ALL ... SELECT 1 FROM dual` because Oracle 23 rejects multi-row VALUES
* combined with the JDBC driver's implicit RETURNING (RETURN_GENERATED_KEYS) handling
* with `ORA: returning clause is not allowed with INSERT and Table Value Constructor`.
* emit `INSERT INTO ... SELECT ... FROM dual UNION ALL ...` (see OracleModel).
*/
public array function $bulkInsertSQL(
required string tableName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ component extends="wheels.databaseAdapters.Base" output=false {
required boolean parameterize,
string $primaryKey = ""
) {
// An INSERT that supplies its own primary-key value may need IDENTITY_INSERT
// (#3647). This runs before the BoxLang branch below on purpose: a wrapped
// statement no longer starts with INSERT INTO, so no SCOPE_IDENTITY() is
// appended — correctly, since the caller already has the key.
if (
Len(Trim(arguments.$primaryKey))
&& IsSimpleValue(arguments.sql[1])
&& Left(arguments.sql[1], 11) == "INSERT INTO"
) {
arguments.sql = $identityInsertSQL(sql = arguments.sql, primaryKey = arguments.$primaryKey);
}

// Same-batch identity retrieval for engines whose query result carries no
// driver-supplied generated key (currently BoxLang). SCOPE_IDENTITY() is
// batch-scoped, so it must ride in the INSERT's own batch; Base.$executeQuery
Expand Down Expand Up @@ -196,6 +208,67 @@ component extends="wheels.databaseAdapters.Base" output=false {
return $performQuery(argumentCollection = arguments);
}

/**
* SQL Server rejects an explicit value for an IDENTITY column unless
* IDENTITY_INSERT is ON for the table, so `create(id = 41, ...)` failed here while
* every other supported database accepts it (#3647). This wraps an INSERT whose
* column list includes a primary-key column (the caller supplied that value) in the
* ON/OFF pair. Three rules shape the wrapper:
*
* - One batch. IDENTITY_INSERT is session-scoped, so the ON, the INSERT and the OFF
* must share a connection; a single statement guarantees that whether or not the
* caller opened a transaction.
* - Guarded by the catalog. The ON/OFF only runs when one of those key columns is
* in sys.identity_columns. A natural or UUID key has no identity property, and
* SET IDENTITY_INSERT on such a table is an error.
* - No TRY/CATCH. The OFF must run even when the INSERT fails: with inlined values
* (parameterize=false) the batch runs as-is and the setting outlives it, so the
* pooled connection's next insert into the table would fail. A constraint
* violation only ends its own statement, so the trailing OFF still runs.
* Re-raising from a CATCH block (THROW) does not work: Lucee drops an error raised
* after a batch's first result, so a duplicate key came back as success.
*
* Returns the SQL array unchanged when no primary-key column is supplied.
*
* @sql The INSERT statement as a `$querySetup()` SQL array.
* @primaryKey The table's primary-key column name(s).
*/
public array function $identityInsertSQL(required array sql, required string primaryKey) {
// Parse the INSERT's column list from the fragment strings only (the values are
// param structs), the same parse $identitySelect uses.
local.text = "";
for (local.part in arguments.sql) {
if (IsSimpleValue(local.part)) {
local.text &= local.part;
}
}
local.insertColumns = $parseInsertColumnList(local.text);

local.names = "";
for (local.key in ListToArray(arguments.primaryKey)) {
local.column = $stripIdentifierQuotes(Trim(local.key));
if (ListFindNoCase(local.insertColumns, local.column)) {
local.names = ListAppend(local.names, "N'" & Replace(local.column, "'", "''", "all") & "'");
}
}
if (!Len(local.names)) {
return arguments.sql;
}

// "INSERT INTO [table] (" -> "[table]"
local.table = Trim(SpanExcluding(Mid(arguments.sql[1], 12, Len(arguments.sql[1])), "("));
local.tableLiteral = Replace(local.table, "'", "''", "all");

ArrayPrepend(
arguments.sql,
"DECLARE @wheelsIdentityInsert bit = CASE WHEN EXISTS (SELECT 1 FROM sys.identity_columns"
& " WHERE object_id = OBJECT_ID(N'" & local.tableLiteral & "') AND name IN (" & local.names & "))"
& " THEN 1 ELSE 0 END; IF @wheelsIdentityInsert = 1 SET IDENTITY_INSERT " & local.table & " ON;"
);
ArrayAppend(arguments.sql, "; IF @wheelsIdentityInsert = 1 SET IDENTITY_INSERT " & local.table & " OFF;");
return arguments.sql;
}

/**
* Acquire a SQL Server application lock using sp_getapplock.
* The lock is scoped to the current session.
Expand Down
5 changes: 5 additions & 0 deletions vendor/wheels/databaseAdapters/Oracle/OracleModel.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ component extends="wheels.databaseAdapters.Base" output=false {
* default is evaluated per row. It is also the shape `$upsertSQL` below already
* uses for its MERGE source, including the alias-the-first-branch-only detail.
*
* Oracle rejects RETURNING after `INSERT ... SELECT` too (ORA-03048), so this
* statement must also reach the driver without a generated-key request. insertAll()
* runs it with `$captureResult=false`, which drops the cfquery `result` attribute —
* the thing that makes Lucee request generated keys (#3653).
*
* Uses parameterized values via `$buildBulkParam` — never interpolates user data
* into SQL.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ interface {
* @dataSource Override datasource.
* @$primaryKey Primary key for identity retrieval.
* @$debugName Debug/logging name.
* @$captureResult False when the caller reads neither the result nor a generated key (bulk paths).
* @return Struct with query and result metadata.
*/
public struct function $performQuery(
Expand All @@ -66,7 +67,8 @@ interface {
numeric offset,
string dataSource,
string $primaryKey,
string $debugName
string $debugName,
boolean $captureResult
);

/**
Expand Down
10 changes: 8 additions & 2 deletions vendor/wheels/model/bulk.cfm
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,13 @@
propertyInfo = variables.wheels.class.properties
);

// Nothing here reads the result or a generated key, so don't request one:
// on Lucee that asks the driver for generated keys, and the Oracle driver
// then appends a RETURNING clause Oracle rejects (#3653).
variables.wheels.class.adapter.$querySetup(
parameterize = arguments.parameterize,
sql = local.sql
sql = local.sql,
$captureResult = false
);

local.totalInserted += (local.batchEnd - local.batchStart + 1);
Expand Down Expand Up @@ -143,9 +147,11 @@
propertyInfo = variables.wheels.class.properties
);

// Same as insertAll(): no result or generated key is read (#3653).
variables.wheels.class.adapter.$querySetup(
parameterize = arguments.parameterize,
sql = local.sql
sql = local.sql,
$captureResult = false
);

local.totalUpserted += (local.batchEnd - local.batchStart + 1);
Expand Down
Loading
Loading