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 | 20220930102342.852ad903b9c0418048c07fe5@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
|
Список | pgsql-hackers |
Hi, On Tue, 9 Aug 2022 00:21:02 +0900 Yugo NAGATA <nagata@sraoss.co.jp> wrote: > 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. I forward it to the hackers list because the patch is to fix comments. Also, I'll register it to commitfest. The past discussion is here. https://www.postgresql.org/message-id/flat/17434-d9f7a064ce2a88a3%40postgresql.org > > > > 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> -- Yugo NAGATA <nagata@sraoss.co.jp>
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Tom LaneДата:
Сообщение: Re: longfin and tamandua aren't too happy but I'm not sure why