BUG #16867: savepoints vs. commit and chain
The following bug has been logged on the website:
Bug reference: 16867
Logged by: Arthur Nascimento
Email address: tureba@gmail.com
PostgreSQL version: 13.2
Operating system: CentOS
Description:
Hi all,
I'm seeing an unexpected behavior when using savepoints along commit and
chain. I couldn't find a mention of it in the docs and the regression tests
seem to not cover it.
On a trivial transaction, I might do:
=# begin;
*=# commit and chain;
*=# -- In this point I'm inside a second transaction
However, if the first transaction contained any unreleased savepoints, the
chain does not get me to a second transaction:
=# begin;
*=# savepoint foo;
*=# commit and chain;
=# -- In this point I'm not inside a second transaction
I first noticed it when using psql's ON_ERROR_ROLLBACK, but it can be
reproduced without it, as shown above.
I also thought it might be psql's fault at not knowing that COMMIT can also
return the state PQTRANS_INTRANS in
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/bin/psql/common.c;h=6f507104f464fde615fb7628f195b0e2149b40ee;hb=HEAD#l1349
But even trying a local fix (by adding COMMIT to the possible commands) to
that didn't quite solve it, so there might be something else.
Let me know what I might be missing in this.
Thanks,
Arthur Nascimento
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
Attachments:
psql-savepoint-cac.txttext/plain; charset=US-ASCII; name=psql-savepoint-cac.txtDownload+5-4
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 transactionI 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 transactionSo 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
Attachments:
savepoint_and_chain.patchtext/plain; charset=UTF-8; name=savepoint_and_chain.patch; x-mac-creator=0; x-mac-type=0Download+58-0
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