Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .ai/wheels/cross-engine-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,14 @@ if (isMySQLFamily) {

CockroachDB is in CI but marked as soft-fail — test failures are logged as warnings, not build failures. Controlled by `SOFT_FAIL_DBS` in `.github/workflows/tests.yml`.

### Oracle — Multi-Row INSERT and RETURNING Incompatibility

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.

`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.

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`.

## Testing Across Engines

### Local Test Procedure
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo

### Fixed

- `model.insertAll()` on Oracle no longer errors with `ORA: returning clause is not allowed with INSERT and Table Value Constructor` (and the related `ORA: no statement parsed` follow-on). The bulk-insert SQL was always emitted as the SQL-standard multi-row table value constructor — `INSERT INTO t (cols) VALUES (?,?), (?,?), ...` — which Oracle 23 rejects in combination with the JDBC driver's implicit `RETURN_GENERATED_KEYS` handling (the driver expands `RETURN_GENERATED_KEYS` into a `RETURNING ROWID` clause, and Oracle 23 disallows `RETURNING` paired with multi-row VALUES). Bulk-insert SQL generation moved off the model mixin (`vendor/wheels/model/bulk.cfc::$buildBulkInsertSQL`, removed) onto the database adapter (`$bulkInsertSQL` on `databaseAdapters/Base.cfc`, mirroring the existing `$upsertSQL` pattern), so adapters can override per-engine. `databaseAdapters/Oracle/OracleModel.cfc` overrides it to emit Oracle's idiomatic multi-row form — `INSERT ALL INTO t (cols) VALUES (...) INTO t (cols) VALUES (...) SELECT 1 FROM dual` — which neither uses the table value constructor nor triggers the RETURNING expansion. Non-Oracle adapters (MySQL, Postgres, SQLite, H2, SQL Server, CockroachDB) keep the standard multi-row VALUES shape unchanged. The migrator-rename "Closed statement" error in the same compat-matrix run is a separate Oracle JDBC lifecycle issue and remains tracked under the parent issue (#2745)
- `addColumnOptionsSpec` now branches on `adapter.adapterName() == "MySQL"` for the `text` + non-empty default assertion, matching the existing `isPostgresFamily` carve-out. MySQL's `MySQLMigrator.optionsIncludeDefault` returns false for `text` / `mediumtext` / `longtext` / `float`, so the Abstract `addColumnOptions` short-circuits the entire DEFAULT clause for those types — emitting `NULL` rather than `DEFAULT '<value>'`. The spec previously asserted `toInclude("DEFAULT")` unconditionally and failed on every MySQL leg of the compat matrix (lucee6/mysql, lucee7/mysql, boxlang/mysql). The MySQL adapter's `optionsIncludeDefault` doc-comment now also explains the legacy pre-8.0.13 TEXT/BLOB constraint that motivates the suppression and references the spec contract. Follow-up to #2661/#2669
- `CockroachDBModel` now overrides `$supportsAdvisoryLocks()` to return `false`, so the four `lockingSpec` `withAdvisoryLock` tests skip cleanly on CockroachDB instead of erroring with `CockroachDB does not support advisory locks.`. The PR that introduced the capability flag (#2670) claimed CockroachDB in its CHANGELOG entry but never added the override — CockroachDB inherits from `PostgreSQLModel`, which reports `true`, so the spec's `beforeEach` skip-guard never fired and the four specs proceeded to call `$acquireAdvisoryLock`, which the adapter throws from by design. Compat-matrix legs `lucee6/cockroachdb`, `lucee7/cockroachdb`, and `boxlang/cockroachdb` now report 4 skips where they previously reported 4 errors. No spec changes needed — the capability-flag layer added in #2670 already does the right thing once the flag is correct (#2743)
- `wheels.middleware.Cors` now short-circuits unmatched `OPTIONS` preflight requests at the dispatch layer, preserving the legacy `set(allowCorsRequests=true)` contract under the new middleware pipeline. Previously, `$findMatchingRoute()` ran before middleware, so a preflight against a path that only declared `POST` (or any non-`OPTIONS` verb) 404'd with `Wheels.RouteNotFound` before the CORS middleware's preflight branch could fire — leaving the middleware strictly less capable than the 3.x global setting it was meant to replace and breaking cross-origin `POST`/`PUT`/`PATCH`/`DELETE` from configured browsers. `Dispatch.$request()` now checks for an `OPTIONS` verb plus a `wheels.middleware.Cors` instance in the global pipeline and, if both are present, runs the pipeline against a no-op core handler before route matching. Dispatch behavior for `OPTIONS` without CORS middleware (still 404s) and for non-`OPTIONS` verbs (still routed normally) is unchanged (#2703)
Expand Down
53 changes: 53 additions & 0 deletions vendor/wheels/databaseAdapters/Base.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,59 @@ 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`.
*/
public array function $bulkInsertSQL(
required string tableName,
required array columns,
required array validProperties,
required array records,
required numeric batchStart,
required numeric batchEnd,
required struct propertyInfo
) {
local.sql = [];

local.colList = "";
for (local.col in arguments.columns) {
if (Len(local.colList)) {
local.colList &= ", ";
}
local.colList &= $quoteIdentifier(local.col);
}

ArrayAppend(local.sql, "INSERT INTO #arguments.tableName# (#local.colList#) VALUES ");

local.propCount = ArrayLen(arguments.validProperties);
for (local.r = arguments.batchStart; local.r <= arguments.batchEnd; local.r++) {
if (local.r > arguments.batchStart) {
ArrayAppend(local.sql, ", ");
}
ArrayAppend(local.sql, "(");
for (local.p = 1; local.p <= local.propCount; local.p++) {
if (local.p > 1) {
ArrayAppend(local.sql, ", ");
}
local.propName = arguments.validProperties[local.p];
local.val = StructKeyExists(arguments.records[local.r], local.propName) ? arguments.records[local.r][local.propName] : "";
ArrayAppend(local.sql, $buildBulkParam(
value = local.val,
propName = local.propName,
propertyInfo = arguments.propertyInfo
));
}
ArrayAppend(local.sql, ")");
}

return local.sql;
}

/**
* Generates database-specific UPSERT SQL as an array compatible with `$querySetup()`.
* Base implementation throws an error — each adapter must override with its own syntax.
Expand Down
59 changes: 59 additions & 0 deletions vendor/wheels/databaseAdapters/Oracle/OracleModel.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,65 @@ component extends="wheels.databaseAdapters.Base" output=false {
return """#UCase(arguments.name)#""";
}

/**
* Oracle bulk insert using `INSERT ALL INTO ... SELECT 1 FROM dual`.
*
* The default Base adapter shape — `INSERT INTO t (cols) VALUES (?,?), (?,?), ...`
* (SQL standard table value constructor) — was rejected on Oracle 23 with
* `ORA: returning clause is not allowed with INSERT and Table Value Constructor`.
* The CFML engine's `cfquery` for INSERT statements implicitly sets
* `Statement.RETURN_GENERATED_KEYS`, which the Oracle JDBC driver translates into a
* RETURNING clause — and Oracle 23 does not permit RETURNING with multi-row VALUES.
*
* `INSERT ALL` is the Oracle-idiomatic multi-row insert form, doesn't trigger the
* RETURNING-clause expansion, and works on every Oracle version Wheels targets.
* Uses parameterized values via `$buildBulkParam` — never interpolates user data
* into SQL.
*/
public array function $bulkInsertSQL(
required string tableName,
required array columns,
required array validProperties,
required array records,
required numeric batchStart,
required numeric batchEnd,
required struct propertyInfo
) {
local.sql = [];

local.colList = "";
for (local.col in arguments.columns) {
if (Len(local.colList)) {
local.colList &= ", ";
}
local.colList &= $quoteIdentifier(local.col);
}

ArrayAppend(local.sql, "INSERT ALL");

local.propCount = ArrayLen(arguments.validProperties);
for (local.r = arguments.batchStart; local.r <= arguments.batchEnd; local.r++) {
ArrayAppend(local.sql, " INTO #arguments.tableName# (#local.colList#) VALUES (");
for (local.p = 1; local.p <= local.propCount; local.p++) {
if (local.p > 1) {
ArrayAppend(local.sql, ", ");
}
local.propName = arguments.validProperties[local.p];
local.val = StructKeyExists(arguments.records[local.r], local.propName) ? arguments.records[local.r][local.propName] : "";
ArrayAppend(local.sql, $buildBulkParam(
value = local.val,
propName = local.propName,
propertyInfo = arguments.propertyInfo
));
}
ArrayAppend(local.sql, ")");
}

ArrayAppend(local.sql, " SELECT 1 FROM dual");

return local.sql;
}

/**
* Oracle upsert using MERGE with USING (SELECT ... FROM dual UNION ALL ...) source.
* Uses parameterized values via $buildBulkParam — never interpolates user data into SQL.
Expand Down
53 changes: 4 additions & 49 deletions vendor/wheels/model/bulk.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@ component {
for (local.batchStart = 1; local.batchStart <= local.totalRecords; local.batchStart += local.batchSize) {
local.batchEnd = Min(local.batchStart + local.batchSize - 1, local.totalRecords);

local.sql = $buildBulkInsertSQL(
local.sql = variables.wheels.class.adapter.$bulkInsertSQL(
tableName = $quotedTableName(),
columns = local.mapped.columns,
validProperties = local.mapped.validProperties,
records = arguments.records,
batchStart = local.batchStart,
batchEnd = local.batchEnd
batchEnd = local.batchEnd,
propertyInfo = variables.wheels.class.properties
);

variables.wheels.class.adapter.$querySetup(
Expand Down Expand Up @@ -196,53 +198,6 @@ component {
return {columns: local.columns, validProperties: local.validProperties};
}

/**
* Builds the SQL array for a multi-row INSERT statement.
* Returns an array compatible with the adapter's `$querySetup()`.
*/
public array function $buildBulkInsertSQL(
required array columns,
required array validProperties,
required array records,
required numeric batchStart,
required numeric batchEnd
) {
local.sql = [];

local.colList = "";
for (local.col in arguments.columns) {
if (Len(local.colList)) {
local.colList &= ", ";
}
local.colList &= $quoteColumn(local.col);
}

ArrayAppend(local.sql, "INSERT INTO #$quotedTableName()# (#local.colList#) VALUES ");

local.propCount = ArrayLen(arguments.validProperties);
for (local.r = arguments.batchStart; local.r <= arguments.batchEnd; local.r++) {
if (local.r > arguments.batchStart) {
ArrayAppend(local.sql, ", ");
}
ArrayAppend(local.sql, "(");
for (local.p = 1; local.p <= local.propCount; local.p++) {
if (local.p > 1) {
ArrayAppend(local.sql, ", ");
}
local.propName = arguments.validProperties[local.p];
local.val = StructKeyExists(arguments.records[local.r], local.propName) ? arguments.records[local.r][local.propName] : "";
ArrayAppend(local.sql, variables.wheels.class.adapter.$buildBulkParam(
value = local.val,
propName = local.propName,
propertyInfo = variables.wheels.class.properties
));
}
ArrayAppend(local.sql, ")");
}

return local.sql;
}

/**
* Adds `createdAt` and `updatedAt` timestamps to bulk record arrays when the model
* is configured for automatic timestamping.
Expand Down
Loading
Loading