fix(model): resolve cockroachdb bulk-ops and locking test failures (#2106) (#2206)

* fix(model): resolve cockroachdb bulk-ops and locking test failures (#2106)

Two CockroachDB-only core-test regressions, 10 failures total.

Bug 1 (6 failures in bulkOperationsSpec): CockroachDBModel.$querySetup
unconditionally appended `RETURNING <primaryKey>` to any `INSERT INTO`
statement. bulk.cfc's insertAll/upsertAll invoke $querySetup without
passing $primaryKey (default ""), so emitted SQL ended in a bare
`RETURNING `, which CockroachDB parses as a syntax error near EOF.
Guard the append: skip when $primaryKey is empty/whitespace, and skip
when any SQL chunk contains `ON CONFLICT` (upsert statements). The
single-row INSERT + $identitySelect path on PostgreSQL is unaffected
because it lives on PostgreSQLModel, not CockroachDBModel, and in
the CockroachDB single-row path $primaryKey is populated.

Bug 2 (4 failures in lockingSpec): the spec was missing the
_isCockroachDB skip shim that 8 sibling specs already declare.
CockroachDBModel correctly throws Wheels.AdvisoryLockNotSupported
(parity with Oracle/H2). Added the shim and `if (_isCockroachDB) return;`
to each withAdvisoryLock test. The forUpdate describes are left intact
since CockroachDB supports SELECT ... FOR UPDATE.

SQLite suite: 3250 passed locally. CI compat-matrix against real
CockroachDB will validate the actual fix.

Closes #2106

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(docs): correct claude.md ci soft-fail claim

SOFT_FAIL_DBS in .github/workflows/compat-matrix.yml (lines 389, 519)
is empty (""); CockroachDB is currently hard-gated in CI, not
soft-fail as previously documented. Update the reference paragraph
to reflect reality and explain how to mark a database as soft-fail
in the future.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: bpamiri

CLAUDE.md

@@ -609,7 +609,7 @@ curl -s "http://localhost:62023/wheels/core/tests?db=mysql&format=json" | \
 - **`private` mixin functions not integrated**: `$integrateComponents()` only copies `public` methods into model/controller objects. ALL helper functions in mixin CFCs (`vendor/wheels/model/*.cfc`, view helpers, etc.) MUST use `public` access. Use `$` prefix for internal scope instead of `private` keyword. BoxLang handles this differently, so `private` may pass BoxLang tests but fail Lucee/Adobe.
 
 ### CI soft-fail databases
-CockroachDB is marked as soft-fail in `.github/workflows/tests.yml` — failures are logged as warnings but don't block the build. The `SOFT_FAIL_DBS` variable controls this. Remove a database from the list once its tests are fixed.
+`SOFT_FAIL_DBS` in `.github/workflows/compat-matrix.yml` (lines 389, 519) is currently empty (`""`) — **all databases, including CockroachDB, are hard-gated in CI**. To mark a database as soft-fail (failures logged as warnings but not blocking the build), add it to `SOFT_FAIL_DBS` in both locations. Remove a database from the list once its tests are fixed.
 
 ### Cleanup
 ```bash

vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc

@@ -65,6 +65,11 @@ component extends="wheels.databaseAdapters.PostgreSQL.PostgreSQLModel" output=fa
 	 * Override query setup to append RETURNING clause to INSERTs.
 	 * CockroachDB does not support pg_get_serial_sequence()/currval(),
 	 * so the RETURNING clause is the correct way to retrieve generated keys.
+	 *
+	 * Skip the RETURNING append on bulk / upsert paths: bulk.cfc's insertAll
+	 * and upsertAll call $querySetup without $primaryKey (default empty), and
+	 * upsert statements already include ON CONFLICT clauses. In both cases,
+	 * appending `RETURNING ` produces syntactically invalid SQL.
 	 */
 	public struct function $querySetup(
 		required array sql,
@@ -74,7 +79,18 @@ component extends="wheels.databaseAdapters.PostgreSQL.PostgreSQLModel" output=fa
 		string $primaryKey = ""
 	) {
 		if (Left(arguments.sql[1], 11) == "INSERT INTO") {
-			ArrayAppend(arguments.sql, "RETURNING #arguments.$primaryKey#");
+			local.shouldAppendReturning = Len(Trim(arguments.$primaryKey)) > 0;
+			if (local.shouldAppendReturning) {
+				for (local.chunk in arguments.sql) {
+					if (IsSimpleValue(local.chunk) && ReFindNoCase("ON[[:space:]]+CONFLICT", local.chunk)) {
+						local.shouldAppendReturning = false;
+						break;
+					}
+				}
+			}
+			if (local.shouldAppendReturning) {
+				ArrayAppend(arguments.sql, "RETURNING #arguments.$primaryKey#");
+			}
 		}
 		$convertMaxRowsToLimit(args = arguments);
 		$removeColumnAliasesInOrderClause(args = arguments);

vendor/wheels/tests/specs/model/lockingSpec.cfc

@@ -3,17 +3,20 @@ component extends="wheels.WheelsTest" {
 	function run() {
 
 		g = application.wo
+		var _isCockroachDB = CreateObject("component", "wheels.migrator.Migration").init().adapter.adapterName() == "CockroachDB";
 
 		describe("Tests that withAdvisoryLock", () => {
 
 			it("executes callback and returns result", () => {
+				if (_isCockroachDB) return;
 				local.result = g.model("author").withAdvisoryLock(name="test_lock_1", callback=function() {
 					return 42;
 				});
 				expect(local.result).toBe(42);
 			})
 
 			it("releases lock even when callback throws an exception", () => {
+				if (_isCockroachDB) return;
 				local.exceptionThrown = false;
 				try {
 					g.model("author").withAdvisoryLock(name="test_lock_2", callback=function() {
@@ -32,13 +35,15 @@ component extends="wheels.WheelsTest" {
 			})
 
 			it("accepts a custom timeout argument", () => {
+				if (_isCockroachDB) return;
 				local.result = g.model("author").withAdvisoryLock(name="test_lock_timeout", timeout=5, callback=function() {
 					return "locked";
 				});
 				expect(local.result).toBe("locked");
 			})
 
 			it("safely handles lock names with single quotes", () => {
+				if (_isCockroachDB) return;
 				// Regression guard: verify lock names are parameterized, not interpolated.
 				// On SQLite this is a no-op, but the call must not throw a SQL syntax error
 				// regardless of adapter. With proper parameterization, "O'Brien" is fine.