Detect cached plan failures by error message instead of source function - #403
Merged
rafiss merged 2 commits intoAug 18, 2026
Conversation
Refine the handling of FEATURE_NOT_SUPPORTED errors from the PG gem by matching the error message instead of relying on the source function. This adjustment accommodates CockroachDB's unique error reporting while maintaining compatibility with PostgreSQL's behavior.
Reference the adapter's CACHED_PLAN_HEURISTIC constant from the tests instead of duplicating the magic string, so the tests can't drift from the heuristic they verify. Add an end-to-end test that triggers a real cached-plan failure against a live server: prime a prepared `SELECT *`, add a column to change the result type, then re-run the statement and assert the adapter evicts the stale statement and transparently retries. This exercises the real recovery path rather than only the classifier with fabricated errors. Recovery only runs outside a transaction, so the test is excluded from the suite's wrapping transaction. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
rafiss
approved these changes
Aug 18, 2026
rafiss
left a comment
rafiss
left a comment
Contributor
There was a problem hiding this comment.
thanks for your contribution! i added an additional commit with an integration test and will merge once that's green.
Merged
rafiss
added a commit
that referenced
this pull request
Aug 19, 2026
- Disabled schema_locked by default on connect for CockroachDB v25.3+ and unlocked tables around batched foreign key changes (#404) - Improved error classification by detecting cached plan failures from the error message instead of the source function (#403) - Stopped advertising support for restarting database transactions (#398) - Fixed enum columns being misdetected as spatial columns (#396) Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
|
@rafiss would you also do the version bump tag for |
rafiss
added a commit
to rafiss/activerecord-cockroachdb-adapter
that referenced
this pull request
Aug 21, 2026
The 7-2-stable test suite had not run in CI for ~1 year, and upstream
Rails 7.2 added test cases that the adapter does not satisfy:
- QuotingTest#test_quote_{integer,big_decimal,rational}: the adapter
intentionally quotes all Numeric values as strings (see quoting.rb,
CVE-2022-44566), so rewrite the expected values via CopyCat, mirroring
what master already does.
- InvertibleMigrationTest#test_migrate_revert_add_unique_constraint_with_invalid_option:
CockroachDB cannot drop a UNIQUE constraint via ALTER TABLE DROP
CONSTRAINT on the v23.2/v24.1/v24.2 versions in this branch's CI
matrix (cockroachdb/cockroach#42840), so exclude it.
These failures are unrelated to the cockroachdb#403 backport.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
rafiss
added a commit
that referenced
this pull request
Aug 21, 2026
Backport of #403 to the 7.2.x branch. Detect cached plan failures (FEATURE_NOT_SUPPORTED) by matching the error message "cached plan must not change result type" instead of the PG_DIAG_SOURCE_FUNCTION. CockroachDB's raising function is an internal detail that has moved (runExecBuilder before cockroachdb/cockroach#164406, execBind after), so source-function matching no longer detects the error. Adapted to the 7.2.x method signature, which receives the ActiveRecord error and inspects its #cause. The live-server recovery test from #403 is omitted because it depends on the exclude_from_transactional_tests helper that does not exist on this branch. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
rafiss
added a commit
that referenced
this pull request
Aug 21, 2026
The 7-2-stable test suite had not run in CI for ~1 year, and upstream
Rails 7.2 added test cases that the adapter does not satisfy:
- QuotingTest#test_quote_{integer,big_decimal,rational}: the adapter
intentionally quotes all Numeric values as strings (see quoting.rb,
CVE-2022-44566), so rewrite the expected values via CopyCat, mirroring
what master already does.
- InvertibleMigrationTest#test_migrate_revert_add_unique_constraint_with_invalid_option:
CockroachDB cannot drop a UNIQUE constraint via ALTER TABLE DROP
CONSTRAINT on the v23.2/v24.1/v24.2 versions in this branch's CI
matrix (cockroachdb/cockroach#42840), so exclude it.
These failures are unrelated to the #403 backport.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Contributor
|
@klaiv v7.2.4 has now been released with this fix: https://rubygems.org/gems/activerecord-cockroachdb-adapter/versions/7.2.4 |
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
3 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.
Since cockroachdb/cockroach#164406, the extended protocol raises
cached plan must not change result typeduring Bind, fromexecBind- butis_cached_plan_failure?matchesPG_DIAG_SOURCE_FUNCTION == "runExecBuilder", so on CockroachDB 26.2 the match fails,ActiveRecord::PreparedStatementCacheExpiredis never raised, and Rails apps lose the evict-and-retry recovery after schema changes. We hit this in production when a migration added a column to a hot table.This PR keeps the SQLSTATE check and matches the error message instead of the source function. The reasoning:
RevalidateCachedQuery->runExecBuilder->execBind), breaking silently each time. The message has been byte-identical in PostgreSQL since 8.3 and drivers such as pgx assert the exact string, so it is the most stable field on the wire.lc_messages. That concern doesn't transfer: CockroachDB pinslc_messagesas a compat no-op and never localizes, so the message match is locale-safe here.Verified against a live CockroachDB: the wire error is exactly
SQLSTATE 0A000/PG_DIAG_MESSAGE_PRIMARY "cached plan must not change result type", inside and outside transactions. Tests cover detection from both known source functions, a wrapped message, non-matching0A000errors, and the rescue path.Companion server-side proposal: cockroachdb/cockroach#173374 pins the error's Routine to
RevalidateCachedQuery(what PostgreSQL reports), which would let stock Rails detection work and make this override deletable long-term. This PR is the fix that covers servers that exist today.