Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands - Mailing list pgsql-bugs
| From | David G. Johnston |
|---|---|
| Subject | Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands |
| Date | |
| Msg-id | CAKFQuwa9JQgKgg3Djrx=7p1Phhj68QEq0TWYG72Xv-nPz61r0w@mail.gmail.com Whole thread Raw |
| In response to | Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands
(Tom Lane <tgl@sss.pgh.pa.us>)
|
| List | pgsql-bugs |
On Fri, Jul 15, 2022 at 2:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thu, Jul 14, 2022 at 5:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmm, that one seems to have slipped past me. I agree it doesn't
>> look good. But why isn't the PreventInTransactionBlock() check
>> blocking the command from even starting?
> I assume because pgbench never sends a BEGIN command so the create database
> sees itself in an implicit transaction and happily goes about its business,
> expecting the system to commit its work immediately after it says it is
> done.
Yeah. Upon inspection, the fundamental problem here is that in extended
query protocol we typically don't issue finish_xact_command() until we
get a Sync message. So even though everything looks kosher when
PreventInTransactionBlock() runs, the client can send another statement
which will be executed in the same transaction, risking trouble.
Here's a draft patch to fix this. We basically just need to force
finish_xact_command() in the same way as we do for transaction control
statements. I considered using the same technology as the code uses
for transaction control --- that is, statically check for the types of
statements that are trouble --- but after reviewing the set of callers
of PreventInTransactionBlock() I gave that up as unmaintainable.
This seems like too narrow a fix though. The fact that a sync message is the thing causing the commit of the implicit transaction in the extended query protocol has been exposed as a latent bug in the system by the introduction of the Pipeline functionality in libpq that relies on the "should" in message protocol's:
"At completion of each series of extended-query messages, the frontend should issue a Sync message. This parameterless message causes the backend to close the current transaction if it's not inside a BEGIN/COMMIT transaction block (“close” meaning to commit if no error, or roll back if error)." [1]
However, the implicit promise of the extended query protocol, which only allows one command to be executed at a time, is that each command, no matter whether it must execute "outside of a transaction", that executes in the implicit transaction block will commit at the end of the command.
I don't see needing to update simple_query_exec to recognize this flag, if it survives, so long as we describe the flag as an implementation detail related to the extended query protocol promise to commit implicit transactions regardless of when the sync command arrives.
Plus, the simple query protocol doesn't have the same one command per transaction promise. Any attempts at equivalency between the two really doesn't have a strong foundation to work from. I could see that code comment you wrote being part of the commit message for why exec_simple_query was not touched but I don't find any particular value in having it remain as presented. If anything, a comment like that would be README scoped describing the differences between the simply and extended protocol.
David J.
pgsql-bugs by date: