Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands

Hi,

Thank you for treating this bug report!

On Tue, 26 Jul 2022 12:14:19 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> > And we are back around to the fact that only by using libpq directly, or
> > via the pipeline feature of pgbench, can one actually exert control over
> > the implicit transaction. The psql and general SQL interface
> > implementation are just going to Sync after each command and so everything
> > looks like one transaction per command to them and only explicit
> > transactions matter.
>
> Right.
>
> > From that, the adjustment you describe above is sufficient for me.
>
> Cool, I'll set about back-patching.
>
> regards, tom lane

I've looked at the commited fix. What I wonder is whether a change in
IsInTransactionBlock() is necessary or not.

+ /*
+ * If we tell the caller we're not in a transaction block, then inform
+ * postgres.c that it had better commit when the statement is done.
+ * Otherwise our report could be a lie.
+ */
+ MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT;
+
return false;

The comment says that is required to prevent the report from being
a lie. Indeed, after this function returns false, it is guaranteed
that following statements are executed in a separate transaction from
that of the current statement. However, there is no guarantee that the
current statement is running in a separate transaction from that of
the previous statements. The only caller of this function is ANALYZE
command, and this is used for the latter purpose. That is, if we are
not in a transaction block, ANALYZE can close the current transaction
and restart another one without affecting previous transactions.
(At least, ANALYZE command seems to assume it.) So,
I think the fix does not seem to make a sense.

In fact, the result of IsInTransactionBlock does not make senses at
all in pipe-line mode regardless to the fix. ANALYZE could commit all
previous commands in pipelining, and this may not be user expected
behaviour. Moreover, before the fix ANALYZE didn't close and open a
transaction if the target is only one table, but after the fix ANALYZE
always issues commit regardless to the number of table.

I am not sure if we should fix it to prevent such confusing behavior
because this breaks back-compatibility, but I prefer to fixing it.

The idea is to start an implicit transaction block if the server receive
more than one Execute messages before receiving Sync as discussed in [1].
I attached the patch for this fix.

If the first command in a pipeline is DDL commands such as CREATE
DATABASE, this is allowed and immediately committed after success, as
same as the current behavior. Executing such commands in the middle of
pipeline is not allowed because the pipeline is regarded as "an implicit
transaction block" at that time. Similarly, ANALYZE in the middle of
pipeline can not close and open transaction.

[1] https://www.postgresql.org/message-id/20220301151704.76adaaefa8ed5d6c12ac3079@sraoss.co.jp

Regards,
Yugo Nagata
--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>