Thread: Re: [BUGS] BUG #1118: Misleading Commit message

Re: [BUGS] BUG #1118: Misleading Commit message

From
Bruce Momjian
Date:
Do we want to add this to TODO:
*  Issue an extra message when COMMIT completes a failed transaction

---------------------------------------------------------------------------

elein wrote:
> 
> 
> 
> On Sun, Mar 28, 2004 at 10:23:26AM -0500, Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > PostgreSQL Bugs List wrote:
> > >> In a block transaction, whether or not there were errors in the transaction 
> > >> issuing a commit; returns a COMMIT confirmation. 
> > 
> > > Uh, the tag indicates the COMMIT completed, not that it was a success.
> > 
> > The current philosophy on command tags is "the tag is the same as the
> > command actually issued".  However we are talking about breaking that
> > rule for EXECUTE, and if we do that, it's hard to say that we should
> > continue to enforce the rule for COMMIT.  It would clearly be useful
> > for a COMMIT that ends a failed transaction to report ROLLBACK instead.
> > 
> > > If we throw an error on a COMMIT, people willl think we did not close
> > > the transacction,
> > 
> > ... which we wouldn't have.  That won't work.
> > 
> > > and if we return a ROLLBACK, they will think they issued a rollback.
> > 
> > Which, in effect, is what they did.  Is it likely that this would break
> > any clients?  The intention of the current design rule is that clients
> > can match the tag against the command they issued, but I don't know of
> > any client code that actually does that.
> > 
> > In any case, we already have some inconsistencies:
> > 
> > regression=# begin;
> > BEGIN
> > regression=# end;
> > COMMIT
> > regression=# begin;
> > BEGIN
> > regression=# abort;
> > ROLLBACK
> > regression=#
> > 
> > so it seems that in some cases we're already following a rule more like
> > "the tag is the same as the command actually *executed*".
> > 
> > I started out not wanting to make this change either, but the more
> > I think about it the harder it is to hold that position.
> > 
> >             regards, tom lane
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 3: if posting/reading through Usenet, please send an appropriate
> >       subscribe-nomail command to majordomo@postgresql.org so that your
> >       message can get through to the mailing list cleanly
> 
> The message could be something like:
> COMMIT: Transaction rolled back due to errors
> 
> That way, it would reflect both the command and the action.
> But I am concerned about the information rather than
> the exact message if someone has better ideas.
> 
> My reason for submitting the bug was as Tom stated:
> > It would clearly be useful
> > for a COMMIT that ends a failed transaction to report ROLLBACK instead.
> 
> A commit that fails does not commit. It rolls back.  
> 
> In general, this would make it friendlier for new people and
> space cadets that don't notice the last statement failed :-)
> 
> Elein
> elein@varlena.com
> 

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


Re: [BUGS] BUG #1118: Misleading Commit message

From
elein
Date:
While Alvarro, et al are messing with transaction syntax
this would be a good time to clarify this message.

--elein

On Fri, Jul 09, 2004 at 10:16:29AM -0400, Bruce Momjian wrote:
> 
> Do we want to add this to TODO:
> 
>     *  Issue an extra message when COMMIT completes a failed transaction
> 
> ---------------------------------------------------------------------------
> 
> elein wrote:
> > 
> > 
> > 
> > On Sun, Mar 28, 2004 at 10:23:26AM -0500, Tom Lane wrote:
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > PostgreSQL Bugs List wrote:
> > > >> In a block transaction, whether or not there were errors in the transaction 
> > > >> issuing a commit; returns a COMMIT confirmation. 
> > > 
> > > > Uh, the tag indicates the COMMIT completed, not that it was a success.
> > > 
> > > The current philosophy on command tags is "the tag is the same as the
> > > command actually issued".  However we are talking about breaking that
> > > rule for EXECUTE, and if we do that, it's hard to say that we should
> > > continue to enforce the rule for COMMIT.  It would clearly be useful
> > > for a COMMIT that ends a failed transaction to report ROLLBACK instead.
> > > 
> > > > If we throw an error on a COMMIT, people willl think we did not close
> > > > the transacction,
> > > 
> > > ... which we wouldn't have.  That won't work.
> > > 
> > > > and if we return a ROLLBACK, they will think they issued a rollback.
> > > 
> > > Which, in effect, is what they did.  Is it likely that this would break
> > > any clients?  The intention of the current design rule is that clients
> > > can match the tag against the command they issued, but I don't know of
> > > any client code that actually does that.
> > > 
> > > In any case, we already have some inconsistencies:
> > > 
> > > regression=# begin;
> > > BEGIN
> > > regression=# end;
> > > COMMIT
> > > regression=# begin;
> > > BEGIN
> > > regression=# abort;
> > > ROLLBACK
> > > regression=#
> > > 
> > > so it seems that in some cases we're already following a rule more like
> > > "the tag is the same as the command actually *executed*".
> > > 
> > > I started out not wanting to make this change either, but the more
> > > I think about it the harder it is to hold that position.
> > > 
> > >             regards, tom lane
> > > 
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 3: if posting/reading through Usenet, please send an appropriate
> > >       subscribe-nomail command to majordomo@postgresql.org so that your
> > >       message can get through to the mailing list cleanly
> > 
> > The message could be something like:
> > COMMIT: Transaction rolled back due to errors
> > 
> > That way, it would reflect both the command and the action.
> > But I am concerned about the information rather than
> > the exact message if someone has better ideas.
> > 
> > My reason for submitting the bug was as Tom stated:
> > > It would clearly be useful
> > > for a COMMIT that ends a failed transaction to report ROLLBACK instead.
> > 
> > A commit that fails does not commit. It rolls back.  
> > 
> > In general, this would make it friendlier for new people and
> > space cadets that don't notice the last statement failed :-)
> > 
> > Elein
> > elein@varlena.com
> > 
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)


Re: [BUGS] BUG #1118: Misleading Commit message

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Do we want to add this to TODO:
>     *  Issue an extra message when COMMIT completes a failed transaction

No --- it's (a) wordy and (b) not responsive to the original complaint,
which was that a client that examines command completion tags will be
easily misled.  We should either change the command tags or do nothing.
        regards, tom lane


Re: [BUGS] BUG #1118: Misleading Commit message

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Do we want to add this to TODO:
> >     *  Issue an extra message when COMMIT completes a failed transaction
> 
> No --- it's (a) wordy and (b) not responsive to the original complaint,
> which was that a client that examines command completion tags will be
> easily misled.  We should either change the command tags or do nothing.

I am not excited about changing the command tag.

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


Re: [BUGS] BUG #1118: Misleading Commit message

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>>> Do we want to add this to TODO:
>>> *  Issue an extra message when COMMIT completes a failed transaction
>> 
>> No --- it's (a) wordy and (b) not responsive to the original complaint,
>> which was that a client that examines command completion tags will be
>> easily misled.  We should either change the command tags or do nothing.

> I am not excited about changing the command tag.

I was not either to start with, but the more I think about it, the more
I think it would be a good idea.
        regards, tom lane


Re: [BUGS] BUG #1118: Misleading Commit message

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >>> Do we want to add this to TODO:
> >>> *  Issue an extra message when COMMIT completes a failed transaction
> >> 
> >> No --- it's (a) wordy and (b) not responsive to the original complaint,
> >> which was that a client that examines command completion tags will be
> >> easily misled.  We should either change the command tags or do nothing.
> 
> > I am not excited about changing the command tag.
> 
> I was not either to start with, but the more I think about it, the more
> I think it would be a good idea.

What tag would we use?  ABORT?

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


Re: [BUGS] BUG #1118: Misleading Commit message

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>>> I am not excited about changing the command tag.
>> 
>> I was not either to start with, but the more I think about it, the more
>> I think it would be a good idea.

> What tag would we use?  ABORT?

No, ROLLBACK, which is what you get when you give the "expected"
command.

regression=# begin;
BEGIN
regression=# select 1/0;
ERROR:  division by zero
regression=# abort;        -- or rollback;
ROLLBACK

regression=# begin;
BEGIN
regression=# select 1/0;
ERROR:  division by zero
regression=# commit;
COMMIT

I think the above is fairly misleading; it would be better to say
ROLLBACK to indicate that we had in fact canceled the transaction.
        regards, tom lane


Re: [BUGS] BUG #1118: Misleading Commit message

From
Jan Wieck
Date:
On 7/10/2004 10:54 AM, Bruce Momjian wrote:
> Tom Lane wrote:
>> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> > Do we want to add this to TODO:
>> >     *  Issue an extra message when COMMIT completes a failed transaction
>> 
>> No --- it's (a) wordy and (b) not responsive to the original complaint,
>> which was that a client that examines command completion tags will be
>> easily misled.  We should either change the command tags or do nothing.
> 
> I am not excited about changing the command tag.
> 

Either changing the command tag or let COMMIT of an aborted transaction 
fail (and stay in aborted transaction state). Those are the only two 
clean ways to communicate to the client "no, I cannot commit".


Jan

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #



Re: [BUGS] BUG #1118: Misleading Commit message

From
Tom Lane
Date:
Jan Wieck <JanWieck@Yahoo.com> writes:
> Either changing the command tag or let COMMIT of an aborted transaction 
> fail (and stay in aborted transaction state). Those are the only two 
> clean ways to communicate to the client "no, I cannot commit".

Well, the latter would *certainly* create compatibility problems, so
I don't find it preferable ...
        regards, tom lane