Fix BEGIN/COMMIT/ROLLBACK silently failing under server cursor mode - #10321
Fix BEGIN/COMMIT/ROLLBACK silently failing under server cursor mode#10321dpage wants to merge 1 commit into
Conversation
…mode (pgadmin-org#8991) execute_void() blindly reused whatever cursor was cached for the connection, which under "server cursor" mode is the named/server-side AsyncDictServerCursor left over from the last SELECT. A named cursor's execute() always wraps the statement as `DECLARE ... CURSOR FOR <query>`, which cannot express a transaction-control statement, so BEGIN/COMMIT/ROLLBACK silently failed (failing one step earlier still, on a `prepare` keyword the server-side cursor's execute() doesn't accept at all) and the exception was swallowed by the background query thread. The transaction was therefore never actually committed or rolled back, and the next poll() picked up the previous query's leftover column info, which is what made the result grid appear instead of the Messages tab. Run the statement through a throwaway plain cursor instead, leaving the cached server-side cursor untouched, and clear the stale column info so poll() correctly reports no result set.
|
Warning Review limit reached
Next review available in: 6 minutes Limit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
Reported as a UI glitch (the result grid stays visible instead of
switching to the Messages tab after Commit/Rollback with "server
cursor" mode on), but the root cause is more serious: under server
cursor mode, BEGIN/COMMIT/ROLLBACK never actually reached the database.
execute_void()reuses whatever cursor is cached on the connection,which under server cursor mode is the named/server-side cursor left
over from the last
SELECT. A named cursor'sexecute()always wrapsthe statement as
DECLARE ... CURSOR FOR <query>, which can't expressa transaction-control statement (
DECLARE ... CURSOR FOR COMMITis asyntax error) — and it actually failed one step earlier still, on a
prepare=keyword the server-side cursor'sexecute()doesn't acceptat all (
TypeError: keyword not supported: prepare). That exceptionwas swallowed by a blanket
except Exceptionin the background querythread, so the statement silently never ran, leaving the transaction
open with no error shown to the user. The empty grid was just the
visible fallout: the next
/pollpicked up the previous query'sleftover column info instead of reporting "no result set".
This routes BEGIN/COMMIT/ROLLBACK through a throwaway plain cursor
instead of the cached server-side one when server cursor mode is
active, and clears the stale column info so
poll()correctly reportsno result set afterwards.
Fixes #8991.
Test plan
connection (both the
prepareTypeErrorand the underlyingDECLARE ... CURSOR FOR COMMITsyntax error), and confirmed the fix'sapproach (a plain
connection.cursor()alongside an open namedcursor) commits correctly and reports
description is Noneafterwards.
test_execute_void_server_cursor.py, covering COMMIT andROLLBACK with a cached server-side cursor: asserts the statement runs
on a plain cursor (not the cached server one) and that stale column
info/row count are cleared. Confirmed it fails without the fix and
passes with it.
python regression/runtests.py --pkg utils.driver.psycopg3.tests.test_execute_void_server_cursorpasses.