Обсуждение: Re: BUG #16867: savepoints vs. commit and chain
On Mon, 15 Feb 2021 at 17:54, PG Bug reporting form <noreply@postgresql.org> wrote: > On a trivial transaction, I might do: > > =# begin; > *=# commit and chain; > *=# -- In this point I'm inside a second transaction I forgot to mention that this case also works as expected: =# begin; *=# savepoint foo; *=# release foo; *=# commit and chain; *=# -- In this point I'm also inside a second transaction So it's only the unmatched savepoint/release transactions that are an issue. I also attached the change I did to psql locally. But since it didn't solve the issue, it's mostly for curiosity's sake. Thanks, Tureba - Arthur Nascimento
Вложения
On 2021/02/16 6:47, Arthur Nascimento wrote: > On Mon, 15 Feb 2021 at 17:54, PG Bug reporting form > <noreply@postgresql.org> wrote: >> On a trivial transaction, I might do: >> >> =# begin; >> *=# commit and chain; >> *=# -- In this point I'm inside a second transaction > > I forgot to mention that this case also works as expected: > > =# begin; > *=# savepoint foo; > *=# release foo; > *=# commit and chain; > *=# -- In this point I'm also inside a second transaction > > So it's only the unmatched savepoint/release transactions that are an issue. > > I also attached the change I did to psql locally. LGTM. > But since it didn't > solve the issue, it's mostly for curiosity's sake. In the server side, ISTM that CommitTransactionCommand() needs to handle the COMMIT AND CHAIN in TBLOCK_SUBCOMMIT case, but it forgot to do that. Patch attached. I'm not sure if this is a bug or an intentional behavior. Probably we need to look at the past discussion about AND CHAIN feature. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Вложения
Hi, Fujii-san, On Tue, 16 Feb 2021 at 01:49, Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > In the server side, ISTM that CommitTransactionCommand() needs to handle > the COMMIT AND CHAIN in TBLOCK_SUBCOMMIT case, but it forgot to do that. > Patch attached. I'm not sure if this is a bug or an intentional behavior. > Probably we need to look at the past discussion about AND CHAIN feature. I can confirm that your patch solved it for me. Thanks for looking into it. Tureba - Arthur Nascimento
On 2021/02/17 3:03, Arthur Nascimento wrote: > Hi, Fujii-san, > > On Tue, 16 Feb 2021 at 01:49, Fujii Masao <masao.fujii@oss.nttdata.com> wrote: >> In the server side, ISTM that CommitTransactionCommand() needs to handle >> the COMMIT AND CHAIN in TBLOCK_SUBCOMMIT case, but it forgot to do that. >> Patch attached. I'm not sure if this is a bug or an intentional behavior. >> Probably we need to look at the past discussion about AND CHAIN feature. > > I can confirm that your patch solved it for me. Thanks for looking into it. Thanks for testing the patch! As far as I read the past discussion about chain transaction, I could not find any mention that current behavior that you reported is intentional. Barring any objection, I will commit the patch that you wrote for psql and the patch I wrote. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On 2/18/21 2:58 PM, Fujii Masao wrote: > > > On 2021/02/17 3:03, Arthur Nascimento wrote: >> Hi, Fujii-san, >> >> On Tue, 16 Feb 2021 at 01:49, Fujii Masao >> <masao.fujii@oss.nttdata.com> wrote: >>> In the server side, ISTM that CommitTransactionCommand() needs to handle >>> the COMMIT AND CHAIN in TBLOCK_SUBCOMMIT case, but it forgot to do that. >>> Patch attached. I'm not sure if this is a bug or an intentional >>> behavior. >>> Probably we need to look at the past discussion about AND CHAIN feature. >> >> I can confirm that your patch solved it for me. Thanks for looking >> into it. > > Thanks for testing the patch! > > As far as I read the past discussion about chain transaction, > I could not find any mention that current behavior that you reported > is intentional. > > Barring any objection, I will commit the patch that you wrote > for psql and the patch I wrote. No objection from me. According to the standard, a COMMIT should destroy all savepoints and terminate the transaction, even if AND CHAIN is specified. -- Vik Fearing
On 2021/02/18 23:10, Vik Fearing wrote: > On 2/18/21 2:58 PM, Fujii Masao wrote: >> >> >> On 2021/02/17 3:03, Arthur Nascimento wrote: >>> Hi, Fujii-san, >>> >>> On Tue, 16 Feb 2021 at 01:49, Fujii Masao >>> <masao.fujii@oss.nttdata.com> wrote: >>>> In the server side, ISTM that CommitTransactionCommand() needs to handle >>>> the COMMIT AND CHAIN in TBLOCK_SUBCOMMIT case, but it forgot to do that. >>>> Patch attached. I'm not sure if this is a bug or an intentional >>>> behavior. >>>> Probably we need to look at the past discussion about AND CHAIN feature. >>> >>> I can confirm that your patch solved it for me. Thanks for looking >>> into it. >> >> Thanks for testing the patch! >> >> As far as I read the past discussion about chain transaction, >> I could not find any mention that current behavior that you reported >> is intentional. >> >> Barring any objection, I will commit the patch that you wrote >> for psql and the patch I wrote. > > No objection from me. According to the standard, a COMMIT should > destroy all savepoints and terminate the transaction, even if AND CHAIN > is specified. You imply that the standard says that COMMIT AND CHAIN should just terminate the transaction if there are savepoints defined, i.e., should not start new transaction? Since I can (maybe wrongly) interpret your comment like that, please let me confirm what the standard says just in case. I was thinking that COMMIT AND CHAIN should destroy all the savepoints, terminate the transaction and start new transaction with the same transaction characteristics immediately. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On 2/19/21 5:02 AM, Fujii Masao wrote: > > > On 2021/02/18 23:10, Vik Fearing wrote: >> >> No objection from me. According to the standard, a COMMIT should >> destroy all savepoints and terminate the transaction, even if AND CHAIN >> is specified. > > You imply that the standard says that COMMIT AND CHAIN should just > terminate > the transaction if there are savepoints defined, i.e., should not start new > transaction? Since I can (maybe wrongly) interpret your comment like that, > please let me confirm what the standard says just in case. The COMMIT terminates the transaction, the AND CHAIN starts a new one. > I was thinking that COMMIT AND CHAIN should destroy all the savepoints, > terminate the transaction and start new transaction with the same > transaction > characteristics immediately. Your thinking is correct! -- Vik Fearing
On 2021/02/19 17:29, Vik Fearing wrote: > On 2/19/21 5:02 AM, Fujii Masao wrote: >> >> >> On 2021/02/18 23:10, Vik Fearing wrote: >>> >>> No objection from me. According to the standard, a COMMIT should >>> destroy all savepoints and terminate the transaction, even if AND CHAIN >>> is specified. >> >> You imply that the standard says that COMMIT AND CHAIN should just >> terminate >> the transaction if there are savepoints defined, i.e., should not start new >> transaction? Since I can (maybe wrongly) interpret your comment like that, >> please let me confirm what the standard says just in case. > > The COMMIT terminates the transaction, the AND CHAIN starts a new one. > >> I was thinking that COMMIT AND CHAIN should destroy all the savepoints, >> terminate the transaction and start new transaction with the same >> transaction >> characteristics immediately. > > Your thinking is correct! Thanks! I pushed the patches. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION