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

"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

  1. Rethink handling of [Prevent|Is]InTransactionBlock in pipeline mode.

  2. Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.

  3. Force immediate commit after CREATE DATABASE etc in extended protocol.