8.0b4: COMMIT outside of a transaction echoes ROLLBACK

just wondering:

test=> select version();
version
------------------------------------------------------------------------------------------
PostgreSQL 8.0.0beta4 on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
3.3.3 (SuSE Linux)
(1 row)

test=> begin;
BEGIN
test=> commit;
COMMIT
test=> commit;
WARNING: there is no transaction in progress
ROLLBACK

Is there any reason ROLLBACK and not COMMIT is echoed here?

Ian Barwick
barwick(at)gmail(dot)com

Ian Barwick wrote:
> just wondering:
>
> test=> select version();
> version
> ------------------------------------------------------------------------------------------
> PostgreSQL 8.0.0beta4 on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
> 3.3.3 (SuSE Linux)
> (1 row)
>
> test=> begin;
> BEGIN
> test=> commit;
> COMMIT
> test=> commit;
> WARNING: there is no transaction in progress
> ROLLBACK
>
> Is there any reason ROLLBACK and not COMMIT is echoed here?

Because the transaction was not committed, but rather rolled back.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

On Tue, 26 Oct 2004 21:42:19 -0400 (EDT), Bruce Momjian
<pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
> Ian Barwick wrote:
>
>
> > just wondering:
> >
> > test=> select version();
> > version
> > ------------------------------------------------------------------------------------------
> > PostgreSQL 8.0.0beta4 on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
> > 3.3.3 (SuSE Linux)
> > (1 row)
> >
> > test=> begin;
> > BEGIN
> > test=> commit;
> > COMMIT
> > test=> commit;
> > WARNING: there is no transaction in progress
> > ROLLBACK
> >
> > Is there any reason ROLLBACK and not COMMIT is echoed here?
>
> Because the transaction was not committed, but rather rolled back.

Aha. It had me a little confused because between the first COMMIT and
the second there were several screens of data, and I wasn't sure if
I'd issued the first COMMIT. Seeing ROLLBACK made me unsure whether I
was still in a transaction which had in just been rolled back.

Pre 8.0 versions echo COMMIT in this situation.

Thanks

Ian Barwick
barwick(at)gmail(dot)com

On Tue, 2004-10-26 at 21:42 -0400, Bruce Momjian wrote:
> > test=> begin;
> > BEGIN
> > test=> commit;
> > COMMIT
> > test=> commit;
> > WARNING: there is no transaction in progress
> > ROLLBACK
> >
> > Is there any reason ROLLBACK and not COMMIT is echoed here?
>
> Because the transaction was not committed, but rather rolled back.

It's still a misleading message; in those circumstances, how about
returning "NO ACTION" instead?

--
Oliver Elphick olly(at)lfix(dot)co(dot)uk
Isle of Wight http://www.lfix.co.uk/oliver
GPG: 1024D/A54310EA 92C8 39E7 280E 3631 3F0E 1EC0 5664 7A2F A543 10EA
========================================
"If a man abide not in me, he is cast forth as a
branch, and is withered; and men gather them, and cast
them into the fire, and they are burned."
John 15:6

Oliver Elphick wrote:
> On Tue, 2004-10-26 at 21:42 -0400, Bruce Momjian wrote:
> > > test=> begin;
> > > BEGIN
> > > test=> commit;
> > > COMMIT
> > > test=> commit;
> > > WARNING: there is no transaction in progress
> > > ROLLBACK
> > >
> > > Is there any reason ROLLBACK and not COMMIT is echoed here?
> >
> > Because the transaction was not committed, but rather rolled back.
>
> It's still a misleading message; in those circumstances, how about
> returning "NO ACTION" instead?

Uh, it took a lot of discussion to agree on ROLLBACK. It would take
even more discussion to add a new tag return value.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Oliver Elphick wrote:
>> On Tue, 2004-10-26 at 21:42 -0400, Bruce Momjian wrote:
>>> test=> begin;
>>> BEGIN
>>> test=> commit;
>>> COMMIT
>>> test=> commit;
>>> WARNING: there is no transaction in progress
>>> ROLLBACK
>>
>> It's still a misleading message; in those circumstances, how about
>> returning "NO ACTION" instead?

> Uh, it took a lot of discussion to agree on ROLLBACK. It would take
> even more discussion to add a new tag return value.

I don't care for "NO ACTION" either. However, the prior discussion had
to do with what to echo in the case that you are saying COMMIT in a
failed transaction. I don't think anyone thought about this particular
corner case, viz COMMIT outside any transaction. I think you could make
a reasonable argument that the tag should remain COMMIT for this case,
since we do not consider it an error.

On the other hand, it's also a pretty minor issue, and if it turns out
to require a lot of code rejiggering to make it do that, I'd not think
it worthwhile.

Comments?

regards, tom lane

On Wed, Oct 27, 2004 at 04:21:53PM -0400, Tom Lane wrote:

> On the other hand, it's also a pretty minor issue, and if it turns out
> to require a lot of code rejiggering to make it do that, I'd not think
> it worthwhile.

Patch attached. It passes the regression tests. It shouldn't have
secondary effects, but please test.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, CSLewis)

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> On Wed, Oct 27, 2004 at 04:21:53PM -0400, Tom Lane wrote:
>> On the other hand, it's also a pretty minor issue, and if it turns out
>> to require a lot of code rejiggering to make it do that, I'd not think
>> it worthwhile.

> Patch attached. It passes the regression tests. It shouldn't have
> secondary effects, but please test.

Wouldn't it be better to just stay in TBLOCK_STARTED state, as if the
COMMIT were just some random utility command?

In any case, the comment right above this needs adjustment ...

regards, tom lane

On Wed, Oct 27, 2004 at 09:29:21PM -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > On Wed, Oct 27, 2004 at 04:21:53PM -0400, Tom Lane wrote:
> >> On the other hand, it's also a pretty minor issue, and if it turns out
> >> to require a lot of code rejiggering to make it do that, I'd not think
> >> it worthwhile.
>
> > Patch attached. It passes the regression tests. It shouldn't have
> > secondary effects, but please test.
>
> Wouldn't it be better to just stay in TBLOCK_STARTED state, as if the
> COMMIT were just some random utility command?

It's the same thing, because CommitTransactionCommand acts identically
either way. I changed it anyway because it seems simpler.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
Licensee shall have no right to use the Licensed Software
for productive or commercial use. (Licencia de StarOffice 6.0 beta)

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> On Wed, Oct 27, 2004 at 09:29:21PM -0400, Tom Lane wrote:
>> Wouldn't it be better to just stay in TBLOCK_STARTED state, as if the
>> COMMIT were just some random utility command?

> It's the same thing, because CommitTransactionCommand acts identically
> either way. I changed it anyway because it seems simpler.

Patch applied.

regards, tom lane

On Sat, 30 Oct 2004 16:45:22 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > On Wed, Oct 27, 2004 at 09:29:21PM -0400, Tom Lane wrote:
> >> Wouldn't it be better to just stay in TBLOCK_STARTED state, as if the
> >> COMMIT were just some random utility command?
>
> > It's the same thing, because CommitTransactionCommand acts identically
> > either way. I changed it anyway because it seems simpler.
>
> Patch applied.

Many thanks for this. I appreciate it's a fairly trivial issue, but
seeing the word "ROLLBACK" when a commit, or at least a non-operation
were expected, can do nasty things to one's blood pressure.

Ian Barwick
barwick(at)gmail(dot)net