Repository navigation
Tell the session about DISCARD - #121
Merged
Merged
Conversation
`DISCARD ALL` is what pgbouncer sends between pooled clients, so every pooled deployment sends it, and a session with its own state to drop had no way to hear about it: the statement was answered entirely by the connection. Session.discard(kind) closes that. TEMPORARY arrives as TEMP, the two being one thing in Postgres, so a session has one spelling to match. All four kinds reach it, including the three the connection does nothing for itself. That is the part worth having: pg_mimic has no plan cache, no sequences and no temp tables, so `DISCARD PLANS` was previously a statement that returned without asking anyone -- while a session may have all three. Returning early would have left three of the four kinds unreachable. Told *before* the connection clears anything, which is the opposite order from set_parameter. That one records and undoes on refusal; undoing a DISCARD ALL would mean snapshotting the whole connection -- settings, statements, portals, savepoints, listeners -- and telling first buys the same atomicity for nothing. A session that refuses leaves everything as it was. 25001 still outranks the hook: `DISCARD ALL` inside a transaction is refused before it means anything, so a session is never told about one that cannot run. The narrow forms are allowed there, as in Postgres. `DISCARD ALL` reaches a session twice over -- once as the statement, then as one set_parameter(name, None, None) per setting still held (#120). The statement first, then what it did. Co-Authored-By: Claude Opus 5 (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
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.
Part 3 of the #35 stack, on top of #119 and #120.
DISCARD ALLis what pgbouncer sends between pooled clients, so every pooled deployment sends it — and a session with its own state to drop had no way to hear about it. The statement was answered entirely by the connection.TEMPORARYarrives asTEMP, the two being one thing in Postgres, so a session has one spelling to match.Two places I deviated from #35's sketch
All four kinds reach the hook, not just
ALL. The connection returns early forPLANS/SEQUENCES/TEMP— "a mimic has none of those to throw away" — which is true of pg_mimic and not of a session, which may have all three. Keeping the early return would have left three of the four kinds unreachable, and they are the ones where the hook is the only thing that could act.Told before the connection clears, not after. #35's sketch says "the connection has already cleared the state it owns". That is right for
set_parameter, which records and undoes on refusal — but undoing aDISCARD ALLmeans snapshotting the whole connection: settings, prepared statements, portals, savepoints, listeners. Telling first buys the same atomicity for nothing, so a session that refuses leaves everything as it was:The two hooks now differ in order, deliberately, and each docstring says why.
What a session sees for DISCARD ALL
The statement first, then what it did — one
set_parameter(name, None, None)per setting still held, from #120:Unchanged
25001still outranks the hook:DISCARD ALLinside a transaction is refused before it means anything, so a session is never told about one that cannot run. The narrow forms are still allowed there, as in Postgres.Tests
11 in a new
tests/test_discard_hook.py— every kind, theTEMPORARY/TEMPequivalence, the narrow forms reaching a session the connection ignores them for, the atomic refusal, both transaction rules, and a session that does not override it being unaffected. Nine fail without the change.Suite: 886 passed, 17 xfailed.
Remaining
_PASSED_THROUGH_SETTINGSreworked — where therole/session_authorizationfinding from An on_execute effect may be awaited #119 lands.