fix: crash on ROLLBACK after error in transaction block#279
Conversation
The post_parse_analyze_hook fired on every SQL statement including ROLLBACK. In an aborted transaction, catalog lookups via SearchSysCache are forbidden, causing an assertion failure (debug builds) or "ResourceOwnerEnlarge called after release started" crash (release builds). Guard the hook with IsTransactionState() to skip catalog lookups when the transaction is not in a valid state.
There was a problem hiding this comment.
LGTM — clean, minimal fix using the standard IsTransactionState() guard.
Extended reasoning...
Overview
This PR fixes a crash when ROLLBACK is issued after an error in a transaction block (#247). The post_parse_analyze_hook fires on every SQL statement including ROLLBACK, and when the transaction is in an aborted state, catalog lookups via SearchSysCache are forbidden. The fix adds a single IsTransactionState() guard around resolve_indexes_in_query() in tp_post_parse_analyze_hook, plus the necessary #include <access/xact.h>. The prev_post_parse_analyze_hook chain call is correctly left unconditional — each hook is responsible for its own state checking.
Security risks
None. The change skips work when the transaction is in an invalid state, which is the correct and safe behavior. No new inputs are accepted, no permissions are changed, and no data paths are altered.
Level of scrutiny
Low scrutiny is appropriate. This is a well-understood PostgreSQL extension pattern — IsTransactionState() is the standard guard for exactly this scenario. The code change is a single conditional wrapping an existing call. The Makefile change just adds the new test to the alphabetically-sorted regression test list.
Other factors
The new abort.sql regression test covers 8 distinct scenarios (basic abort, abort with BM25 index present, abort during CREATE INDEX, abort during DROP TABLE, multiple sequential errors, SAVEPOINT/ROLLBACK TO, error in BM25 query, abort after successful BM25 query), providing thorough coverage for the fix. No bugs were found by the bug hunting system. The PR directly references the issue it fixes (#247).
## Summary - Bugfix release: fix crash on ROLLBACK after error in transaction block (#279) - Version bump from 1.0.0-dev to 0.6.1 - Updated all SQL files and test expected output for new version string ## Testing - CI validates all regression tests, sanitizers, and upgrade paths
Fixes #247.
Summary
post_parse_analyze_hookfires on every SQL statement, includingROLLBACK. When a transaction is in an aborted state (after an error likeSELECT 1/0), catalog lookups viaSearchSysCacheare forbidden. This caused an assertion failure on debug builds and a "ResourceOwnerEnlarge called after release started" crash on release builds.IsTransactionState()to skip catalog lookups when the transaction is not in a valid state.Testing
abort.sqlregression test covering 8 scenarios: basic abort, abort with BM25 index present, abort during CREATE INDEX, abort during DROP TABLE with BM25 index, multiple sequential errors, SAVEPOINT/ROLLBACK TO, error in BM25 query, and abort after successful BM25 query.