Merge pull request #1999 from wheels-dev/peter/cockroachdb-adapter-phase2

fix: CockroachDB adapter phase 2 — fix SQL generation and remove soft-fail

Author: bpamiri

.github/workflows/tests.yml

@@ -386,8 +386,7 @@ jobs:
             fi
 
             # Databases whose failures are logged but don't block CI.
-            # CockroachDB populate.cfm fails to create tables (tracked for fix).
-            SOFT_FAIL_DBS="cockroachdb"
+            SOFT_FAIL_DBS=""
 
             # Track per-database result
             if [ "$HTTP_CODE" = "200" ]; then
@@ -517,7 +516,7 @@ jobs:
           echo "| Database | Result |" >> $GITHUB_STEP_SUMMARY
           echo "|----------|--------|" >> $GITHUB_STEP_SUMMARY
 
-          SOFT_FAIL_DBS="cockroachdb"
+          SOFT_FAIL_DBS=""
           IFS=',' read -ra DBS <<< "${{ steps.db-list.outputs.databases }}"
           for db in "${DBS[@]}"; do
             RESULT_FILE="/tmp/test-results/${{ matrix.cfengine }}-${db}-result.txt"

docs/superpowers/specs/2026-03-27-cockroachdb-adapter-phase2-design.md

@@ -0,0 +1,118 @@
+# CockroachDB Adapter Improvements (Phase 2)
+
+**Issues**: #1972, #1974
+**Goal**: Fix CockroachDB test failures and remove from SOFT_FAIL_DBS in CI
+
+## Context
+
+CockroachDB is marked as soft-fail in CI (`SOFT_FAIL_DBS="cockroachdb"` in `.github/workflows/tests.yml`). Tests run but failures don't block the build. Current results on Lucee 6: **2217 pass, 31 fail, 38 error** out of 507 suites.
+
+Phase 1 (already merged) established the CockroachDB adapter: `CockroachDBModel.cfc` (type mapping, RETURNING clause identity select) and `CockroachDBMigrator.cfc` (native sqlTypes, unique_rowid() PK generation), plus unit and integration tests.
+
+Phase 2 fixes the remaining test failures so CockroachDB becomes a first-class CI target.
+
+## Root Causes
+
+### 1. Missing CockroachDB in adapter name checks (9 SQL errors + ~20 cascading failures)
+
+The SQL generation layer in `model/update.cfc` and `model/sql.cfc` uses hardcoded adapter name lists like `ListFind('PostgreSQL,H2,Oracle,SQLite', adapterName())`. CockroachDB returns `"CockroachDB"` for `adapterName()` and matches **none** of these branches.
+
+The critical failure: `update.cfc:108` — the `UPDATE table SET` prefix is never generated for CockroachDB, producing SQL like `"authorid" = ` (no table, no SET keyword). This causes 9 direct SQL syntax errors and cascading failures in tests that depend on UPDATE operations.
+
+**Locations to fix** (add `CockroachDB` alongside `PostgreSQL`):
+
+| File | Line | Current List |
+|------|------|-------------|
+| `vendor/wheels/model/update.cfc` | 108 | `PostgreSQL,H2,Oracle,SQLite` |
+| `vendor/wheels/model/sql.cfc` | 653 | `PostgreSQL,H2,MicrosoftSQLServer,Oracle,SQLite` |
+| `vendor/wheels/model/sql.cfc` | 664 | `PostgreSQL` |
+| `vendor/wheels/model/sql.cfc` | 701 | `PostgreSQL,H2` |
+
+### 2. unique_rowid() generates large INT8 IDs (3-5 test failures)
+
+CockroachDB's SERIAL uses `unique_rowid()` producing large non-sequential INT8 values (e.g., `1161774559250219009`). Core tests that hardcode `Expected [1]` or `Expected [5]` fail.
+
+Affected tests:
+- `readSpec > findfirst > works` — expects id=1
+- `readSpec > findLastOne > works` — expects id=5
+- `crudSpec > order > is working with maxrows and calculated property` — ID in expected string
+- `sqlSpec > works with numeric operators` — expects cf_sql_integer (gets cf_sql_bigint)
+- `migrationSpec > is changing column` — expects limit 50 (gets 2147483647)
+
+### 3. Transaction behavior differences (12 failures in transactionsSpec)
+
+CockroachDB uses SERIALIZABLE isolation by default and has different SAVEPOINT semantics. Several transaction tests assume READ COMMITTED behavior or specific savepoint rollback mechanics.
+
+## Design
+
+### Part 1: Fix adapter name checks
+
+Mechanical change: add `CockroachDB` to each `ListFind()` call that currently includes `PostgreSQL` in `update.cfc` and `sql.cfc`. CockroachDB uses the PostgreSQL wire protocol and should receive identical SQL generation treatment.
+
+### Part 2: Adapter-specific test suites
+
+**Strategy**: Guard core tests that have CockroachDB-incompatible assumptions, then provide equivalent coverage through CockroachDB-specific test specs.
+
+**Guard pattern** (in core spec files):
+```cfm
+it("expects sequential IDs", () => {
+    if ($isCockroachDB()) return; // covered by CockroachDBCrudSpec
+    // ... test with hardcoded IDs
+});
+```
+
+The `$isCockroachDB()` helper checks the current adapter name. This is the pattern already used in `CockroachDBIntegrationSpec.cfc`.
+
+**New test files** (in `vendor/wheels/tests/specs/database/`):
+
+1. **CockroachDBCrudSpec.cfc** — CRUD lifecycle with non-sequential ID assertions:
+   - Create returns numeric key > 0
+   - Sequential creates produce unique increasing keys
+   - findFirst/findLast work without assuming specific ID values
+   - findAll with maxrows works with large IDs
+
+2. **CockroachDBTransactionSpec.cfc** — Transaction behavior under SERIALIZABLE isolation:
+   - Basic transaction commit/rollback
+   - invokeWithTransaction callback behavior
+   - Nested transaction semantics
+   - deleteAll/updateAll within transactions
+
+3. **CockroachDBTypeSpec.cfc** — Type introspection for CockroachDB-specific types:
+   - SERIAL columns report as cf_sql_bigint (INT8)
+   - STRING type maps correctly
+   - Column metadata returns expected limits
+
+### Part 3: Remove from SOFT_FAIL_DBS
+
+After all fixes verified locally on Lucee 6 + Adobe 2025:
+- Remove `cockroachdb` from `SOFT_FAIL_DBS` on lines 390 and 520 of `.github/workflows/tests.yml`
+
+## Files to modify
+
+| File | Change |
+|------|--------|
+| `vendor/wheels/model/update.cfc` | Add CockroachDB to adapter name list (line 108) |
+| `vendor/wheels/model/sql.cfc` | Add CockroachDB to adapter name lists (lines 653, 664, 701) |
+| `vendor/wheels/tests/specs/model/readSpec.cfc` | Guard CockroachDB-incompatible ID tests |
+| `vendor/wheels/tests/specs/model/crudSpec.cfc` | Guard CockroachDB-incompatible ID/order tests |
+| `vendor/wheels/tests/specs/model/sqlSpec.cfc` | Guard CockroachDB type expectation test |
+| `vendor/wheels/tests/specs/migrator/migrationSpec.cfc` | Guard CockroachDB column limit test |
+| `vendor/wheels/tests/specs/model/transactionsSpec.cfc` | Guard CockroachDB-incompatible transaction tests |
+| `vendor/wheels/tests/specs/database/CockroachDBCrudSpec.cfc` | **New** — CRUD with non-sequential IDs |
+| `vendor/wheels/tests/specs/database/CockroachDBTransactionSpec.cfc` | **New** — SERIALIZABLE transaction tests |
+| `vendor/wheels/tests/specs/database/CockroachDBTypeSpec.cfc` | **New** — Type introspection tests |
+| `.github/workflows/tests.yml` | Remove cockroachdb from SOFT_FAIL_DBS |
+
+## Verification
+
+1. Run Lucee 6 + CockroachDB locally: `curl "http://localhost:60006/wheels/core/tests?db=cockroachdb&format=json"`
+2. Run Adobe 2025 + CockroachDB locally: `curl "http://localhost:62025/wheels/core/tests?db=cockroachdb&format=json"`
+3. Confirm 0 fail, 0 error for CockroachDB across both engines
+4. Run Lucee 6 + H2 to verify no regressions: `curl "http://localhost:60006/wheels/core/tests?db=h2&format=json"`
+5. Push and verify CI passes with CockroachDB as a blocking database
+
+## Open questions
+
+- Some transaction failures may be fixable in the framework (not just test expectations) — investigate after fixing SQL generation to see which persist
+- IN clause failures (`works with IN operator with spaces`) may be a separate SQL generation or CockroachDB dialect issue — investigate after main fix
+- Guard pattern: use inline `adapterName() != "CockroachDB"` check (matches existing pattern in CockroachDBIntegrationSpec.cfc) rather than adding a new base class helper — keeps change minimal

vendor/wheels/databaseAdapters/Base.cfc

@@ -105,10 +105,13 @@ component output=false extends="wheels.Global"{
 		}
 
 		// 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.
 		wheels.id = $identitySelect(
-			primaryKey      = args.primaryKey,
-			queryAttributes = args.queryAttributes,
-			result          = wheels.result
+			primaryKey         = args.primaryKey,
+			queryAttributes    = args.queryAttributes,
+			result             = wheels.result,
+			returningIdentity  = structKeyExists(local, args.debugName) ? local[args.debugName] : ""
 		);
 
 		if (structKeyExists(wheels,"id") && isStruct(wheels.id) && !structIsEmpty(wheels.id)) {
@@ -168,7 +171,8 @@ component output=false extends="wheels.Global"{
 	public any function $identitySelect(
 		required struct queryAttributes,
 		required struct result,
-		required string primaryKey
+		required string primaryKey,
+		any returningIdentity = ""
 	) {
 		local.query = {};
 		local.sql = Trim(arguments.result.sql);

vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc

@@ -10,7 +10,7 @@ component extends="wheels.databaseAdapters.PostgreSQL.PostgreSQLModel" output=fa
 		switch (arguments.type) {
 			case "bool":
 			case "boolean":
-				local.rv = "cf_sql_boolean";
+				local.rv = "cf_sql_bit";
 				break;
 			case "bit":
 			case "varbit":

vendor/wheels/databaseAdapters/MicrosoftSQLServer/MicrosoftSQLServerModel.cfc

@@ -224,7 +224,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
 	public any function $identitySelect(
 		required struct queryAttributes,
 		required struct result,
-		required string primaryKey
+		required string primaryKey,
+		any returningIdentity = ""
 	) {
 		var query = {};
 		local.sql = Trim(arguments.result.sql);

vendor/wheels/databaseAdapters/Oracle/OracleModel.cfc

@@ -94,7 +94,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
 	public any function $identitySelect(
 		required struct queryAttributes,
 		required struct result,
-		required string primaryKey
+		required string primaryKey,
+		any returningIdentity = ""
 	) {
 		var query = {};
 		local.sql = Trim(arguments.result.sql);

vendor/wheels/databaseAdapters/PostgreSQL/PostgreSQLModel.cfc

@@ -124,7 +124,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
 	public any function $identitySelect(
 		required struct queryAttributes,
 		required struct result,
-		required string primaryKey
+		required string primaryKey,
+		any returningIdentity = ""
 	) {
 		var query = {};
 		local.sql = Trim(arguments.result.sql);

vendor/wheels/databaseAdapters/SQLite/SQLiteModel.cfc

@@ -82,7 +82,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
     public any function $identitySelect(
         required struct queryAttributes,
         required struct result,
-        required string primaryKey
+        required string primaryKey,
+        any returningIdentity = ""
     ) {
         var query = {};
         var local = {};

vendor/wheels/model/sql.cfc

@@ -650,7 +650,7 @@ component {
 		// Issue#1273: Added this section to allow included tables to be referenced in the query
 		local.migration = CreateObject("component", "wheels.migrator.Migration").init();
 		local.tempSql = "";
-		if(arguments.include != "" && ListFind('PostgreSQL,H2,MicrosoftSQLServer,Oracle,SQLite', local.migration.adapter.adapterName()) && structKeyExists(arguments, "sql")){
+		if(arguments.include != "" && ListFind('PostgreSQL,CockroachDB,H2,MicrosoftSQLServer,Oracle,SQLite', local.migration.adapter.adapterName()) && structKeyExists(arguments, "sql")){
 			local.tempSql = arguments.sql;
 		}
 		local.whereClause = $whereClause(
@@ -661,7 +661,7 @@ component {
 			useIndex = arguments.useIndex,
 			sql = local.tempSql
 		);
-		if(arguments.include != "" && ListFind('PostgreSQL', local.migration.adapter.adapterName()) && structKeyExists(arguments, "sql")){
+		if(arguments.include != "" && ListFind('PostgreSQL,CockroachDB', local.migration.adapter.adapterName()) && structKeyExists(arguments, "sql")){
 			if(left(arguments.sql[1], 6) == 'UPDATE'){
 				ArrayAppend(arguments.sql, "FROM #arguments.include#");
 			}
@@ -698,7 +698,7 @@ component {
 			// Issue#1273: Added this section to allow included tables to be referenced in the query
 			local.joinclause = "";
 			local.migration = CreateObject("component", "wheels.migrator.Migration").init();
-			if(arguments.include != "" && ListFind('PostgreSQL,H2', local.migration.adapter.adapterName()) && left(arguments.sql[1], 6) == 'UPDATE'){
+			if(arguments.include != "" && ListFind('PostgreSQL,CockroachDB,H2', local.migration.adapter.adapterName()) && left(arguments.sql[1], 6) == 'UPDATE'){
 				for(local.i = 1; local.i<= arrayLen(local.classes); i++){
 					if(structKeyExists(local.classes[local.i], "JOIN")){
 						local.joinclause &= local.classes[local.i].JOIN.Split("ON")[2];

vendor/wheels/model/update.cfc

@@ -105,7 +105,7 @@ component {
 					ArrayAppend(arguments.sql, "UPDATE #$quotedTableName()# SET");
 				}
 			}
-			else if (ListFind('PostgreSQL,H2,Oracle,SQLite', local.migration.adapter.adapterName())){
+			else if (ListFind('PostgreSQL,CockroachDB,H2,Oracle,SQLite', local.migration.adapter.adapterName())){
 				ArrayAppend(arguments.sql, "UPDATE #$quotedTableName()# SET");
 			}
 			local.pos = 0;