Horton

Thread

Commits

  1. Fix psql's ON_ERROR_ROLLBACK so that it handles COMMIT AND CHAIN.

  2. Fix bug in COMMIT AND CHAIN command.

  1. BUG #16867: savepoints vs. commit and chain

    The Post Office <noreply@postgresql.org> — 2021-02-15T20:53:41Z

    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

  2. Re: BUG #16867: savepoints vs. commit and chain

    Arthur Nascimento <tureba@gmail.com> — 2021-02-15T21:47:29Z

    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

  3. Re: BUG #16867: savepoints vs. commit and chain

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2021-02-16T04:49:33Z

    
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

  4. Re: BUG #16867: savepoints vs. commit and chain

    Arthur Nascimento <tureba@gmail.com> — 2021-02-16T18:03:53Z

    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

  5. Re: BUG #16867: savepoints vs. commit and chain

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2021-02-18T13:58:48Z

    
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

  6. Re: BUG #16867: savepoints vs. commit and chain

    Vik Fearing <vik@postgresfriends.org> — 2021-02-18T14:10:32Z

    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

  7. Re: BUG #16867: savepoints vs. commit and chain

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2021-02-19T04:02:42Z

    
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

  8. Re: BUG #16867: savepoints vs. commit and chain

    Vik Fearing <vik@postgresfriends.org> — 2021-02-19T08:29:32Z

    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

  9. Re: BUG #16867: savepoints vs. commit and chain

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2021-02-19T13:05:25Z

    
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