Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands
От | Yugo NAGATA |
---|---|
Тема | Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands |
Дата | |
Msg-id | 20220809002102.b815cd4dad20fdc91dff5ce8@sraoss.co.jp обсуждение исходный текст |
Ответы |
Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands
(Yugo NAGATA <nagata@sraoss.co.jp>)
Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands (Peter Eisentraut <peter.eisentraut@enterprisedb.com>) |
Список | pgsql-hackers |
On Wed, 27 Jul 2022 22:50:55 -0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: > 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. Thank you for your explanation. I understood that IsInTransactionBlock() and PreventInTransactionBlock() share the equivalent assumption. As to ANALYZE, after investigating the code more, I found that setting XACT_FLAGS_NEEDIMMEDIATECOMMIT in IsInTransactionBlock()is needed indeed. That is, some flags in pg_class such as relhasindex can be safely updated only if ANALYZE is not in a transaction block and never rolled back. So, in a pipeline, ANALYZE must be immediately committed. However, I think we need more comments on these functions to clarify what users can expect or not for them. It is ensured that the statement that calls PreventInTransactionBlock() or receives false from IsInTransactionBlock() never be rolled back if it finishes successfully. This can eliminate the harmful influence of non-rollback-able side effects. On the other hand, it cannot ensure that the statement calling these functions is the first or only one in the transaction in pipelining. If there are preceding statements in a pipeline, they are committed in the same transaction of the current statement. The attached patch tries to add comments explaining it on the functions. > > 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. I am not sure if we can think CREATE DATABASE case and ANLALYZE case similarly. First, CREATE DATABASE is one of the commands that cannot be executed inside a transaction block, but ANALYZE can be. So, users would not be able to know ANALYZE in a pipeline causes a commit from the documentation. Second, ANALYZE issues a commit internally in an early stage not only after it finished successfully. For example, even if ANALYZE is failing because a not-existing column name is specified, it issues a commit before checking the column name. This makes more hard to know which statements will be committed and which statements not committed in a pipeline. Also, as you know, there are other commands that issue internal commits. > 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. I don't have no idea how to reduce the complexity explained above and clarify the transactional behavior of pipelining to users other than the fix I proposed in the previous post. However, I also agree that such changing may make some people unhappy. If there is no good way and we would not like to change the behavior, I think it is better to mention the effects of commands that issue internal commits in the documentation at least. Regards, Yugo Nagata -- Yugo NAGATA <nagata@sraoss.co.jp>
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Ibrar AhmedДата:
Сообщение: Get the statistics based on the application name and IP address