Yugo NAGATA <nagata@sraoss.co.jp> writes:
> I've looked at the commited fix. What I wonder is whether a change in
> IsInTransactionBlock() is necessary or not.
I've not examined ANALYZE's dependencies on this closely, but it doesn't
matter really, because I'm not willing to assume that ANALYZE is the
only caller. There could be external modules with stronger assumptions
that IsInTransactionBlock() yielding false provides guarantees equivalent
to PreventInTransactionBlock(). It did before this patch, so I think
it needs to still do so after.
> 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.
This seems pretty much isomorphic to the fact that CREATE DATABASE
will commit preceding steps in the pipeline. That's not great,
I admit; we'd not have designed it like that if we'd had complete
understanding of the behavior at the beginning. But it's acted
like that for a couple of decades now, so changing it seems far
more likely to make people unhappy than happy. The same for
ANALYZE in a pipeline.
> 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.
I'm not going there. If you can persuade some other committer that
this is worth breaking backward compatibility for, fine; the user
complaints will be their problem.
regards, tom lane