From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
Subject: | Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands |
Date: | 2022-07-15 21:06:51 |
Message-ID: | 3498921.1657919211@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
"David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> On Thu, Jul 14, 2022 at 5:37 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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. So
what this does is make PreventInTransactionBlock() set a flag to be
checked later, back in exec_execute_message. I was initially going
to make that be a new boolean global, but I happened to notice the
MyXactFlags variable which seems entirely suited to this use-case.
One thing that I'm dithering over is whether to add a check of the
new flag in exec_simple_query. As things currently stand that would
be redundant, but it seems like doing things the same way in both
of those functions might be more future-proof and understandable.
(Note the long para I added to justify not doing it ;-))
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
ensure-immediate-commit-in-extended-protocol-1.patch | text/x-diff | 4.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-07-15 22:14:07 | Re: Excessive number of replication slots for 12->14 logical replication |
Previous Message | Andres Freund | 2022-07-15 20:25:58 | Re: Makefile.global will override configure parameters if "pgsql" and "postgres" appear anywhere in the source path name |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-07-15 21:07:25 | Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size |
Previous Message | Jacob Champion | 2022-07-15 21:01:53 | Re: [PATCH] Log details for client certificate failures |