Add CockroachDB RETURNING-based identity select and CI test matrix#1896
Draft
zainforbjs wants to merge 8 commits intodevelopfrom
Draft
Add CockroachDB RETURNING-based identity select and CI test matrix#1896zainforbjs wants to merge 8 commits intodevelopfrom
zainforbjs wants to merge 8 commits intodevelopfrom
Conversation
3 tasks
CockroachDB uses the PostgreSQL wire protocol, so some JDBC drivers report "PostgreSQL" as the driver name. Add a SELECT version() fallback in Model.cfc and migrator/Base.cfc to detect CockroachDB when the driver doesn't expose it in database_productname. Also add CockroachDB to adapter name lists in sql.cfc and update.cfc so UPDATE with include works correctly. Co-Authored-By: Zain Ul Abideen <zainforbjs@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CockroachDB doesn't support pg_get_serial_sequence()/currval(), so use a RETURNING clause on INSERTs to retrieve the generated primary key. - CockroachDBModel extends PostgreSQLModel (inherits query helpers) - $querySetup appends RETURNING to INSERT statements - $identitySelect handles generatedKey (Lucee) and returningIdentity (ACF) - Base.cfc passes returningIdentity from cfquery result to adapters - Boolean stays mapped to cf_sql_boolean (CockroachDB strict typing) Co-Authored-By: Zain Ul Abideen <zainforbjs@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add cockroachdb service to compose.yml with init scripts - Add wheelstestdb_cockroachdb datasource to all 8 CFConfig files - Add cockroachdb to CI database list and readiness checks - Add cockroachdb column to test matrix summary grid - Update test runner to accept cockroachdb as valid db param - Update populate.cfm with CockroachDB-specific identity column type - Loosen test assertions for integer types (bigint for CockroachDB keys) - Use readiness loop instead of fixed sleep in start.sh Co-Authored-By: Zain Ul Abideen <zainforbjs@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bpamiri
force-pushed
the
cockroachdb-enhacement
branch
from
c4e8467 to
0f2d61f
Compare
bpamiri
changed the title
Contributor
Wheels Test Matrix
Results for commit ac5a8d8. |
Contributor
Wheels Test Results 50 files + 8 1 errors 4 655 suites +749 35m 53s ⏱️ + 10m 11s For more details on these parsing errors, failures and errors, see this check. Results for commit 6b54f83. ± Comparison against base commit 63a6bdc. ♻️ This comment has been updated with latest results. |
Tests $getType boolean mapping, $generatedKey, $identitySelect with generatedKey/returningIdentity/column-list-guard/non-INSERT guard, and migrator addPrimaryKeyOptions with unique_rowid(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CRUD lifecycle, boolean handling, sequential identity select, and findAll. Guarded to only run when db=cockroachdb. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dation type map $getValidationType mapped CF_SQL_BIT to "boolean" but had no case for CF_SQL_BOOLEAN, causing CockroachDB boolean columns to fall through to "string". This made hasChanged() report false positives when comparing boolean values like "false" against the persisted false. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ance CFML closures execute in their defining scope, not the scope they're assigned to. The injected action closures couldn't find renderText() and renderNothing() because those are controller variables-scope methods. Fix by capturing the controller instance in a local var and calling ctrl.renderText()/ctrl.renderNothing() explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CFML void functions don't assign null to a variable — the variable simply doesn't exist, causing 'variable [RV] doesn't exist' errors. Use IsNull() on the raw function call instead of toBeNull() on a var. Verified: 1979 specs, 0 failures, 0 errors on Lucee 6 + MySQL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes CockroachDB adapter support with proper identity select and full CI coverage.
pg_get_serial_sequence()/currval(), so INSERTs now useRETURNING pkto retrieve generated keys — no race conditionSELECT version()disambiguates whendriver_namereports "PostgreSQL"Changes from original PR
PostgreSQLModel(notBase) — inherits query helpers, only overrides what differscf_sql_boolean(notcf_sql_bit) — CockroachDB enforces strict type checkingPostgreSQLMigratorwithunique_rowid())server.jsonchangecurrval()fallback (unsupported by CockroachDB)sleep 10in start.sh with proper readiness loopCloses #1971, closes #1973
🤖 Generated with Claude Code