Skip to content

fix(model): MSSQL same-batch and Oracle CURRVAL identity fallbacks plus $identitySelect dedup #3010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bpamiri merged 1 commit into from
Jun 11, 2026
Merged

fix(model): MSSQL same-batch and Oracle CURRVAL identity fallbacks plus $identitySelect dedup #3010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions changelog.d/db-adapter-identity-fallbacks-and-dedup.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- SQL Server adapter: same-batch `SCOPE_IDENTITY()` retrieval replaces the session-scoped `@@IDENTITY` identity fallback on engines without driver generated keys, and paginated queries no longer drop their `GROUP BY` clause ([#2906](https://github.com/wheels-dev/wheels/issues/2906), [#2961](https://github.com/wheels-dev/wheels/issues/2961))
- Oracle adapter: identity fallback now resolves the backing identity sequence and uses session-scoped `CURRVAL` instead of the race-prone `MAX(ROWID)` heuristic, and `findAll(order="random")` emits `DBMS_RANDOM.VALUE` instead of the invalid `RANDOM()` ([#2906](https://github.com/wheels-dev/wheels/issues/2906), [#2961](https://github.com/wheels-dev/wheels/issues/2961))
- The six divergent `$identitySelect` copies across database adapters are unified into a Base template method with per-engine `$parseInsertColumnList`/`$lastIdLookup` hooks, so engine workarounds land once ([#2906](https://github.com/wheels-dev/wheels/issues/2906))
115 changes: 98 additions & 17 deletions vendor/wheels/databaseAdapters/Base.cfc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,38 +142,119 @@ component output=false extends="wheels.Global"{
return "generated_key";
}

/**
* Reports whether the current CFML engine is BoxLang. Centralized here so
* the adapters' engine-conditional branches (column-list parsing, identity
* retrieval) share one probe — and so test doubles can override it without
* application state ($engineAdapter() requires an initialized app).
*/
public boolean function $isBoxLangEngine() {
return StructKeyExists(server, "boxlang");
}

/**
* Extract the column list from a rendered INSERT statement (the names
* between the first parenthesis pair), normalized to a plain comma list:
* whitespace and newlines removed, identifier quotes stripped.
* Returns an empty string when no complete parenthesis pair is found.
*/
public string function $parseInsertColumnList(required string sql) {
local.startPar = Find("(", arguments.sql) + 1;
local.endPar = Find(")", arguments.sql);
local.columnList = "";
if (local.startPar > 1 && local.endPar > local.startPar) {
local.rawColumns = Mid(arguments.sql, local.startPar, (local.endPar - local.startPar));
if ($isBoxLangEngine()) {
// BoxLang's ReplaceList behaves differently — use regex to parse the column names.
local.columnList = REReplace(local.rawColumns, "\s*,\s*", ",", "all");
local.columnList = REReplace(local.columnList, "[\r\n]", "", "all");
local.columnList = Trim(local.columnList);
} else {
// Original Lucee / Adobe CF behavior.
local.columnList = ReplaceList(local.rawColumns, "#Chr(10)#,#Chr(13)#, ", ",,");
}
}
// Strip identifier quotes from the column list for comparison.
return $stripIdentifierQuotes(local.columnList);
}

/**
* Called after a query has executed.
* If the query was an INSERT and the generated auto-incrementing primary key is not in the result we get it manually.
* If the primary key was part of the INSERT (i.e. it wasn't auto-incrementing) we don't need to check it though.
* This process is typically needed on non-supported databases (example: H2) and drivers (example: jTDS).
* We return void or a struct containing the key name / value.
*
* Template method: the shared INSERT / already-present-key / bulk-path /
* key-in-column-list guards live here; the engine-specific retrieval is
* delegated to the $lastIdLookup hook each adapter overrides.
*/
public any function $identitySelect(
required struct queryAttributes,
required struct result,
required string primaryKey,
any returningIdentity = ""
) {
local.query = {};
local.sql = Trim(arguments.result.sql);
if (Left(local.sql, 11) == "INSERT INTO" && !StructKeyExists(arguments.result, $generatedKey())) {
local.startPar = Find("(", local.sql) + 1;
local.endPar = Find(")", local.sql);
local.columnList = ReplaceList(
Mid(local.sql, local.startPar, (local.endPar - local.startPar)),
"#Chr(10)#,#Chr(13)#, ",
",,"
);
// Strip identifier quotes from column list for comparison
local.columnList = $stripIdentifierQuotes(local.columnList);
if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
local.rv = {};
query = $query(sql = "SELECT LAST_INSERT_ID() AS lastId", argumentCollection = arguments.queryAttributes);
local.rv[$generatedKey()] = query.lastId;
return local.rv;
}
if (Left(local.sql, 11) != "INSERT INTO" || StructKeyExists(arguments.result, $generatedKey())) {
return;
}

// Bulk operations (insertAll / upsertAll) invoke the shared query path
// without a primary-key hint, because the caller does not consume a
// generated key. Skip the lookup entirely in that case — it would be a
// wasted round-trip at best, and on PostgreSQL it would emit
// `pg_get_serial_sequence(..., '')`, which Postgres rejects with
// `column "" of relation "..." does not exist`.
if (!Len(arguments.primaryKey)) {
return;
}

// If the primary key was part of the INSERT it wasn't auto-generated.
if (ListFindNoCase($parseInsertColumnList(local.sql), ListFirst(arguments.primaryKey))) {
return;
}

// Hook contract: return a simple value (possibly empty) to publish it
// under $generatedKey(); return nothing to publish nothing.
local.id = $lastIdLookup(
queryAttributes = arguments.queryAttributes,
result = arguments.result,
primaryKey = arguments.primaryKey,
returningIdentity = arguments.returningIdentity,
insertSql = local.sql
);
if (!StructKeyExists(local, "id")) {
return;
}
local.rv = {};
local.rv[$generatedKey()] = local.id;
return local.rv;
}

/**
* Engine-specific hook used by the $identitySelect template to retrieve the
* generated key after an INSERT. Adapters override this with their own
* retrieval strategy (sequence CURRVAL, same-batch resultset, driver key…).
*
* Contract: return a simple value (possibly empty) to publish it under
* $generatedKey(); return nothing (void) to publish nothing.
*
* @queryAttributes Struct of cfquery attributes (datasource etc.) for follow-up queries.
* @result The cfquery result struct from the INSERT.
* @primaryKey Primary key column name(s).
* @returningIdentity Resultset produced by the INSERT's own batch, when one exists.
* @insertSql The trimmed SQL of the executed INSERT statement.
*/
public any function $lastIdLookup(
required struct queryAttributes,
required struct result,
required string primaryKey,
any returningIdentity = "",
required string insertSql
) {
local.query = $query(sql = "SELECT LAST_INSERT_ID() AS lastId", argumentCollection = arguments.queryAttributes);
return local.query.lastId;
}

/**
Expand Down
50 changes: 12 additions & 38 deletions vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,50 +120,24 @@ component extends="wheels.databaseAdapters.PostgreSQL.PostgreSQLModel" output=fa
}

/**
* Retrieve the last inserted primary key value.
* Tries multiple strategies: result.generatedKey (Lucee), result.query (ACF),
* and the returningIdentity query result from the RETURNING clause.
* Override the PostgreSQL adapter's $identitySelect hook.
* Tries result.generatedKey (Lucee) and the returningIdentity query result
* from the RETURNING clause appended in $querySetup. Never round-trips to
* the database: CockroachDB does not support pg_get_serial_sequence() /
* currval(), so when neither source is present nothing is published.
*/
public any function $identitySelect(
public any function $lastIdLookup(
required struct queryAttributes,
required struct result,
required string primaryKey,
any returningIdentity = ""
any returningIdentity = "",
required string insertSql
) {
var query = {};
local.sql = Trim(arguments.result.sql);
if (Left(local.sql, 11) != "INSERT INTO" || StructKeyExists(arguments.result, $generatedKey())) {
return;
if (StructKeyExists(arguments.result, "generatedKey")) {
return ListFirst(arguments.result.generatedKey);
}

local.startPar = Find("(", local.sql) + 1;
local.endPar = Find(")", local.sql);
local.columnList = "";
if (local.endPar) {
local.rawColumns = Mid(local.sql, local.startPar, (local.endPar - local.startPar));
if (StructKeyExists(server, "boxlang")) {
local.columnList = REReplace(local.rawColumns, "\s*,\s*", ",", "all");
local.columnList = REReplace(local.columnList, "[\r\n]", "", "all");
local.columnList = Trim(local.columnList);
} else {
local.columnList = ReplaceList(local.rawColumns, "#Chr(10)#,#Chr(13)#, ", ",,");
}
}

// Strip identifier quotes for comparison
local.columnList = $stripIdentifierQuotes(local.columnList);

if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
local.rv = {};
if (StructKeyExists(arguments.result, "generatedKey")) {
query.id = ListFirst(arguments.result.generatedKey);
} else if (IsQuery(arguments.returningIdentity) && arguments.returningIdentity.recordCount) {
query.id = arguments.returningIdentity[arguments.primaryKey][1];
}
if (StructKeyExists(query, "id")) {
local.rv[$generatedKey()] = query.id;
return local.rv;
}
if (IsQuery(arguments.returningIdentity) && arguments.returningIdentity.recordCount) {
return arguments.returningIdentity[arguments.primaryKey][1];
}
}

Expand Down
118 changes: 57 additions & 61 deletions vendor/wheels/databaseAdapters/MicrosoftSQLServer/MicrosoftSQLServerModel.cfc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,21 @@ component extends="wheels.databaseAdapters.Base" output=false {
required boolean parameterize,
string $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
// passes the resulting resultset to $identitySelect as returningIdentity
// (the same plumbing the CockroachDB adapter uses for RETURNING). Bulk paths
// pass no primary-key hint and are skipped.
if (
$isBoxLangEngine()
&& Len(Trim(arguments.$primaryKey))
&& IsSimpleValue(arguments.sql[1])
&& Left(arguments.sql[1], 11) == "INSERT INTO"
) {
ArrayAppend(arguments.sql, ";SELECT SCOPE_IDENTITY() AS lastId");
}

if (StructKeyExists(arguments, "maxrows") && arguments.maxrows > 0) {
if (arguments.maxrows > 0) {
arguments.sql[1] = ReplaceNoCase(arguments.sql[1], "SELECT ", "SELECT TOP #arguments.maxrows# ", "one");
Expand Down Expand Up @@ -137,12 +152,14 @@ component extends="wheels.databaseAdapters.Base" output=false {
local.firstSelect = $columnAlias(list = $tableName(list = local.thirdSelect, action = "remove"), action = "keep");

// We need to add columns from the inner order clause to the select clauses in the inner two queries.
// Strip identifier quotes once up front (and keep the stripped list in sync with appends below)
// instead of re-stripping the whole select list on every loop iteration.
local.thirdSelectStripped = $stripIdentifierQuotes(local.thirdSelect);
local.iEnd = ListLen(local.thirdOrder);
for (local.i = 1; local.i <= local.iEnd; local.i++) {
local.item = ReReplace(ReReplace(ListGetAt(local.thirdOrder, local.i), " ASC\b", ""), " DESC\b", "");
// Strip identifier quotes for comparison since SELECT may have different quoting than ORDER BY
local.itemStripped = $stripIdentifierQuotes(local.item);
local.thirdSelectStripped = $stripIdentifierQuotes(local.thirdSelect);
if (!ListFindNoCase(local.thirdSelectStripped, local.itemStripped) && !ListFindNoCase(local.thirdSelect, local.item)) {
// The test "order_clause_with_paginated_include_and_ambiguous_columns" passes in a complex order (CASE WHEN registration IN ('foo') THEN 0 ELSE 1 END DESC).
// This gets moved up to the SELECT clause to support pagination.
Expand All @@ -153,6 +170,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
}

local.thirdSelect = ListAppend(local.thirdSelect, local.item);
local.thirdSelectStripped = ListAppend(local.thirdSelectStripped, $stripIdentifierQuotes(local.item));
}
if (local.containsGroup) {
local.item = ReReplace(local.item, "[[:space:]]AS[[:space:]][A-Za-z1-9]+", "", "all");
Expand Down Expand Up @@ -190,7 +208,7 @@ component extends="wheels.databaseAdapters.Base" output=false {
if (local.containsGroup) {
local.afterWhere = "GROUP BY " & local.thirdGroup & " ";
}
local.afterWhere = "ORDER BY " & local.thirdOrder & ") AS tmp1 ORDER BY " & local.secondOrder & ") AS tmp2 ORDER BY " & local.firstOrder;
local.afterWhere &= "ORDER BY " & local.thirdOrder & ") AS tmp1 ORDER BY " & local.secondOrder & ") AS tmp2 ORDER BY " & local.firstOrder;
ArrayDeleteAt(arguments.sql, 1);
ArrayDeleteAt(arguments.sql, 1);
ArrayDeleteAt(arguments.sql, ArrayLen(arguments.sql));
Expand Down Expand Up @@ -262,73 +280,51 @@ component extends="wheels.databaseAdapters.Base" output=false {
}

/**
* Override Base adapter's function.
* Override Base adapter's $identitySelect hook.
*/
public any function $identitySelect(
public any function $lastIdLookup(
required struct queryAttributes,
required struct result,
required string primaryKey,
any returningIdentity = ""
any returningIdentity = "",
required string insertSql
) {
var query = {};
local.sql = Trim(arguments.result.sql);
if (Left(local.sql, 11) == "INSERT INTO" && !StructKeyExists(arguments.result, $generatedKey())) {
local.startPar = Find("(", local.sql) + 1;
local.endPar = Find(")", local.sql);
local.columnList = "";
if (local.endPar) {
local.rawColumns = Mid(local.sql, local.startPar, (local.endPar - local.startPar));

// BoxLang compatibility fix - ReplaceList behaves differently
if (StructKeyExists(server, "boxlang")) {
// For BoxLang, use regex to properly parse column names
local.columnList = REReplace(local.rawColumns, "\s*,\s*", ",", "all");
local.columnList = REReplace(local.columnList, "[\r\n]", "", "all");
local.columnList = Trim(local.columnList);
} else {
// Original Lucee/ACF behavior
local.columnList = ReplaceList(
local.rawColumns,
"#Chr(10)#,#Chr(13)#, ",
",,"
);
}
}
// Strip identifier quotes from column list for comparison
local.columnList = $stripIdentifierQuotes(local.columnList);
if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
local.rv = {};

// Prefer the driver-supplied generated key: mssql-jdbc retrieves it in the
// insert's own batch, so it is both scope-safe and trigger-safe — a trigger
// that inserts into another identity table cannot leak its key in here.
// StructKeyExists is case-insensitive, so Lucee's lowercase `generatedkey`
// result key matches. ListFirst because multi-row inserts can return a list.
if (StructKeyExists(arguments.result, "generatedKey") && Len(arguments.result.generatedKey)) {
local.rv[$generatedKey()] = ListFirst(arguments.result.generatedKey);
return local.rv;
}
// Prefer the driver-supplied generated key: mssql-jdbc retrieves it in the
// insert's own batch, so it is both scope-safe and trigger-safe — a trigger
// that inserts into another identity table cannot leak its key in here.
// StructKeyExists is case-insensitive, so Lucee's lowercase `generatedkey`
// result key matches. ListFirst because multi-row inserts can return a list.
if (StructKeyExists(arguments.result, "generatedKey") && Len(arguments.result.generatedKey)) {
return ListFirst(arguments.result.generatedKey);
}

// Last resort — only runs when the driver supplied no generated key
// (currently BoxLang + SQL Server). @@IDENTITY is session-scoped and can
// return a trigger-generated identity instead of ours. The order is NOT
// flipped to prefer SCOPE_IDENTITY() because a standalone
// `SELECT SCOPE_IDENTITY()` executes in its own scope (a batch is a scope)
// and returns NULL — the documented BoxLang empty-value behavior — so the
// reorder would just add a wasted round-trip per insert and still resolve
// via @@IDENTITY. Same-batch retrieval for this fallback is tracked in a
// follow-up issue.
query = $query(sql = "SELECT @@IDENTITY AS lastId", argumentCollection = arguments.queryAttributes);

// Fallback to SCOPE_IDENTITY() if @@IDENTITY returned nothing (other CFML engines).
if (!Len(query.lastId)) {
query = $query(sql = "SELECT SCOPE_IDENTITY() AS lastId", argumentCollection = arguments.queryAttributes);
}
// Same-batch retrieval: when the engine surfaces no driver key, $querySetup
// appended `;SELECT SCOPE_IDENTITY() AS lastId` to the INSERT's own batch
// (SCOPE_IDENTITY() is batch-scoped, so it must ride along — a standalone
// `SELECT SCOPE_IDENTITY()` executes in its own scope and returns NULL).
// Base.$executeQuery pipes the batch's resultset in here as returningIdentity.
if (
IsQuery(arguments.returningIdentity)
&& arguments.returningIdentity.recordCount
&& ListFindNoCase(arguments.returningIdentity.columnList, "lastId")
&& Len(arguments.returningIdentity.lastId[1])
) {
return arguments.returningIdentity.lastId[1];
}

local.rv[$generatedKey()] = query.lastId;
return local.rv;
}
// Absolute last resort — only reached when the multi-statement batch did not
// surface a usable resultset on this engine/driver combo. @@IDENTITY is
// session-scoped and can return a trigger-generated identity from another
// table, but keeping it means a same-batch miss degrades to the pre-fix
// behavior instead of losing the key entirely.
local.query = $query(sql = "SELECT @@IDENTITY AS lastId", argumentCollection = arguments.queryAttributes);

// Fallback to SCOPE_IDENTITY() if @@IDENTITY returned nothing (other CFML engines).
if (!Len(local.query.lastId)) {
local.query = $query(sql = "SELECT SCOPE_IDENTITY() AS lastId", argumentCollection = arguments.queryAttributes);
}

return local.query.lastId;
}

/**
Expand Down
Loading
Loading