Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: Bruce Momjian <bruce@momjian.us>, Yugo Nagata <nagata@sraoss.co.jp>,
PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Date: 2022-07-26T15:08:21Z
Lists: pgsql-bugs, pgsql-hackers
Attachments
- ensure-immediate-commit-in-extended-protocol-2.patch (text/x-diff) patch
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> I guess I am expecting exec_execute_message to have:
> if (completed && use_implicit_block)
> {
> EndImplicitTransactionBlock();
> finish_xact_command();
> } else if (completed) [existing code continues]
The problem with that is "where do we get use_implicit_block from"?
In simple query mode it's set if the simple-query message contains
more than one statement. But the issue we face in extended mode is
precisely that we don't know if the client will try to send another
statement before Sync.
I spent some time thinking about alternative solutions for this.
AFAICS the only other feasible approach is to continue to not do
finish_xact_command() until Sync, but change state so that any
message that tries to do other work will be rejected. But that's
not really at all attractive, for these reasons:
1. Rejecting other message types implies an error (unless we
get REALLY weird), which implies a rollback, which gets us into
the same inconsistent state as a user-issued rollback.
2. Once we've completed the CREATE DATABASE or whatever, we really
have got to commit or we end with inconsistent state. So it does
not seem like a good plan to sit and wait for the client, even if
we were certain that it'd eventually issue Sync. The longer we
sit, the more chance of something interfering --- database shutdown,
network connection drop, etc.
3. This approach winds up throwing errors for cases that used
to work, eg multiple CREATE DATABASE commands before Sync.
The immediate-silent-commit approach doesn't. The only compatibility
break is that you can't ROLLBACK after CREATE DATABASE ... but that's
precisely the case that doesn't work anyway.
Ideally we'd dodge all of this mess by making all our DDL fully
transactional and getting rid of PreventInTransactionBlock.
I'm not sure that will ever happen; but I am sad that so many
new calls of it have been introduced by the logical replication
stuff. (Doesn't look like anybody bothered to teach psql's
command_no_begin() about those, either.) In any case, that's a
long-term direction to pursue, not something that could yield
a back-patchable fix.
Anyway, here's an updated patch, now with docs. I was surprised
to realize that protocol.sgml has no explicit mention of pipelining,
even though extended query protocol was intentionally set up to make
that possible. So I added a <sect2> about that, which provides a home
for the caveat about immediate-commit commands.
regards, tom lane
Commits
-
Rethink handling of [Prevent|Is]InTransactionBlock in pipeline mode.
- f48aa5df4e03 11.19 landed
- ae47f8a9664a 14.7 landed
- 942cc240f959 13.10 landed
- 1cca4a75ffb8 12.14 landed
- 18431ee6f511 15.2 landed
- 20432f873140 16.0 landed
-
Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.
- fec80da849f3 11.19 landed
- e70cd16f2223 15.2 landed
- e613ace1f0d5 16.0 landed
- 8befa05d7889 14.7 landed
- 1949135e79e8 13.10 landed
- 17e9ecac0118 12.14 landed
-
Force immediate commit after CREATE DATABASE etc in extended protocol.
- f92944137cde 16.0 landed
- a0c632c1dea7 15.0 landed
- 9e3e1ac458ab 11.17 landed
- 968b89257b11 12.12 landed
- 964f42aa297b 10.22 landed
- 6c193c2ace32 13.8 landed
- 3e1297a63f76 14.5 landed