A bad behavior under autocommit off mode 
Author Message
 A bad behavior under autocommit off mode

--ELM1048094319-521-1_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII

OK, I have a patch to fix this bug.  The basic problem is that when a
multi-query string is passed to the backend, it is treated as a single
transaction _unless_ a transaction or GUC command appears in the string.
When they appear, a transaction is forced, but the normal transaction
state machine has been bypassed, meaning in:

        SET autocommit TO off; SELECT 1; COMMIT;

when the COMMIT arrives, the transaction state machines hasn't seen the
SELECT because the mechanism is bypassing the state machine to try and
get everything into the same transaction.

This patch removes that "stuff all queries into a single transaction"
behavior and makes them function just like queries arriving separately.
This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
behavior, they just need to wrap BEGIN/COMMIT around the query string.

I could have fixed it with a hack to the transaction state machine, but
this seems like the proper fix.  I never liked that single-transaction
query string behavior anyway.  It seemed too strange.

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

Quote:

> Hi all,

> There seems a bad behavior under autocommit off mode.

>   1) psql -c 'set autocommit to off;select 1;commit'
> causes a WARNING:  COMMIT: no transaction in progress
> whereas
>   2) psql -c 'begin;select 1;commit'
> causes no error/warning.

> Note that the result is the same even when you issue
> the first set/begin command separately using the client
> softwares other than psql.

> The problem here is that the transaction is cancelled
> in case of 1) though no error is reported.
> Shouldn't we avoid the warning and the cancellation ?
> Or should an error be reported instead of the warning ?

> regards,
> Hiroshi Inoue
>     http://www.***.com/

> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command


--
  Bruce Momjian                        |   http://www.***.com/

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

--ELM1048094319-521-1_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain
Content-Disposition: inline; filename="/bjm/diff"

Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.317
diff -c -c -r1.317 postgres.c
*** src/backend/tcop/postgres.c 10 Mar 2003 03:53:51 -0000      1.317
--- src/backend/tcop/postgres.c 19 Mar 2003 16:54:53 -0000
***************
*** 880,899 ****
                                finish_xact_command(true);
                                xact_started = false;
                        }
!               }                                               /* end loop over queries generated from a
!                                                                * parsetree */

!               /*
!                * If this is the last parsetree of the query string, close down
!                * transaction statement before reporting command-complete.  This
!                * is so that any end-of-transaction errors are reported before
!                * the command-complete message is issued, to avoid confusing
!                * clients who will expect either a command-complete message or an
!                * error, not one and then the other.  But for compatibility with
!                * historical Postgres behavior, we do not force a transaction
!                * boundary between queries appearing in a single query string.
!                */
!               if (lnext(parsetree_item) == NIL && xact_started)
                {
                        finish_xact_command(false);
                        xact_started = false;
--- 880,888 ----
                                finish_xact_command(true);
                                xact_started = false;
                        }
!               }       /* end loop over queries generated from a parsetree */

!               if (xact_started)
                {
                        finish_xact_command(false);
                        xact_started = false;

--ELM1048094319-521-1_
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.***.com/

--ELM1048094319-521-1_--



Mon, 05 Sep 2005 01:18:52 GMT
 A bad behavior under autocommit off mode

Quote:

> OK, I have a patch to fix this bug.  The basic problem is that when a
> multi-query string is passed to the backend, it is treated as a single
> transaction _unless_ a transaction or GUC command appears in the string.
> When they appear, a transaction is forced, but the normal transaction
> state machine has been bypassed, meaning in:

>         SET autocommit TO off; SELECT 1; COMMIT;

> when the COMMIT arrives, the transaction state machines hasn't seen the
> SELECT because the mechanism is bypassing the state machine to try and
> get everything into the same transaction.

> This patch removes that "stuff all queries into a single transaction"
> behavior and makes them function just like queries arriving separately.
> This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> behavior, they just need to wrap BEGIN/COMMIT around the query string.

Does the change worth the trouble ?
Please don't break BACKWARD COMPATIBILITY easily.

regards,
Hiroshi Inoue
        http://www.geocities.jp/inocchichichi/psqlodbc/

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html



Mon, 05 Sep 2005 07:52:23 GMT
 A bad behavior under autocommit off mode

Quote:


> > OK, I have a patch to fix this bug.  The basic problem is that when a
> > multi-query string is passed to the backend, it is treated as a single
> > transaction _unless_ a transaction or GUC command appears in the string.
> > When they appear, a transaction is forced, but the normal transaction
> > state machine has been bypassed, meaning in:

> >         SET autocommit TO off; SELECT 1; COMMIT;

> > when the COMMIT arrives, the transaction state machines hasn't seen the
> > SELECT because the mechanism is bypassing the state machine to try and
> > get everything into the same transaction.

> > This patch removes that "stuff all queries into a single transaction"
> > behavior and makes them function just like queries arriving separately.
> > This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> > behavior, they just need to wrap BEGIN/COMMIT around the query string.

> Does the change worth the trouble ?
> Please don't break BACKWARD COMPATIBILITY easily.

It clearly fixes an existing bug, and I asked on general to see if
anyone has any problem with the change.  My guess is that more people
are surprised by the group-string-as-a-single-transaction as people who
use the feature, so I see it as the removal of surprising functionality.

We will mention it in the release notes, and I can even supply a patch
for those who want it kept.  In fact, I can easily make it a compile
option --- the change is only a single conditional test.

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org



Mon, 05 Sep 2005 10:37:58 GMT
 A bad behavior under autocommit off mode

Quote:



> > > OK, I have a patch to fix this bug.  The basic problem is that when a
> > > multi-query string is passed to the backend, it is treated as a single
> > > transaction _unless_ a transaction or GUC command appears in the string.
> > > When they appear, a transaction is forced, but the normal transaction
> > > state machine has been bypassed, meaning in:

> > >         SET autocommit TO off; SELECT 1; COMMIT;

> > > when the COMMIT arrives, the transaction state machines hasn't seen the
> > > SELECT because the mechanism is bypassing the state machine to try and
> > > get everything into the same transaction.

> > > This patch removes that "stuff all queries into a single transaction"
> > > behavior and makes them function just like queries arriving separately.
> > > This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> > > behavior, they just need to wrap BEGIN/COMMIT around the query string.

> > Does the change worth the trouble ?
> > Please don't break BACKWARD COMPATIBILITY easily.

> It clearly fixes an existing bug,

My proposal also fixes the bug though Tom objected to it.

regards,
Hiroshi Inoue
        http://www.geocities.jp/inocchichichi/psqlodbc/

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate

message can get through to the mailing list cleanly



Mon, 05 Sep 2005 11:06:16 GMT
 A bad behavior under autocommit off mode

Quote:

> > > > This patch removes that "stuff all queries into a single transaction"
> > > > behavior and makes them function just like queries arriving separately.
> > > > This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> > > > behavior, they just need to wrap BEGIN/COMMIT around the query string.

> > > Does the change worth the trouble ?
> > > Please don't break BACKWARD COMPATIBILITY easily.

> > It clearly fixes an existing bug,

> My proposal also fixes the bug though Tom objected to it.

Yes, there are other ways to fix this --- but this is the cleanest, and
I think cleans up some surprising functionality.  We cleaned up some
suprises in 7.3 (pg_atoi), and if we don't do cleanups like this, we
accumulate a system that behaves eratically --- as you showed in the
original report of psql -c not working for autocommit off.

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org



Mon, 05 Sep 2005 11:26:21 GMT
 A bad behavior under autocommit off mode

Quote:


> > > > > This patch removes that "stuff all queries into a single transaction"
> > > > > behavior and makes them function just like queries arriving separately.
> > > > > This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> > > > > behavior, they just need to wrap BEGIN/COMMIT around the query string.

> > > > Does the change worth the trouble ?
> > > > Please don't break BACKWARD COMPATIBILITY easily.

> > > It clearly fixes an existing bug,

> > My proposal also fixes the bug though Tom objected to it.

> Yes, there are other ways to fix this --- but this is the cleanest,

I don't think so. Anyway my proposal doesn't break backward
compatibility at least.

regards,
Hiroshi Inoue
        http://www.geocities.jp/inocchichichi/psqlodbc/

---------------------------(end of broadcast)---------------------------



Mon, 05 Sep 2005 11:35:12 GMT
 A bad behavior under autocommit off mode

Quote:



> > > > > > This patch removes that "stuff all queries into a single transaction"
> > > > > > behavior and makes them function just like queries arriving separately.
> > > > > > This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> > > > > > behavior, they just need to wrap BEGIN/COMMIT around the query string.

> > > > > Does the change worth the trouble ?
> > > > > Please don't break BACKWARD COMPATIBILITY easily.

> > > > It clearly fixes an existing bug,

> > > My proposal also fixes the bug though Tom objected to it.

> > Yes, there are other ways to fix this --- but this is the cleanest,

> I don't think so. Anyway my proposal doesn't break backward
> compatibility at least.

If you would like to post your approach, we can take a vote and see
what people want.

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster



Mon, 05 Sep 2005 11:55:52 GMT
 A bad behavior under autocommit off mode

Quote:

> Does the change worth the trouble ?
> Please don't break BACKWARD COMPATIBILITY easily.

If it's important enough, we can provide a GUC option for it.

My guess is that a GUC option isn't needed, but then again we've been
criticized for disregarding backward compatibility in the past...

Cheers,

Neil

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html



Mon, 05 Sep 2005 12:10:21 GMT
 A bad behavior under autocommit off mode

Quote:


> > Does the change worth the trouble ?
> > Please don't break BACKWARD COMPATIBILITY easily.

> If it's important enough, we can provide a GUC option for it.

> My guess is that a GUC option isn't needed, but then again we've been
> criticized for disregarding backward compatibility in the past...

I think our policy is going to be that GUC backward compatibility
options have to be asked for during beta, unless it is obvious.

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html



Mon, 05 Sep 2005 12:14:11 GMT
 A bad behavior under autocommit off mode

Quote:



> > > Does the change worth the trouble ?
> > > Please don't break BACKWARD COMPATIBILITY easily.

> > If it's important enough, we can provide a GUC option for it.

> > My guess is that a GUC option isn't needed, but then again we've been
> > criticized for disregarding backward compatibility in the past...

> I think our policy is going to be that GUC backward compatibility
> options have to be asked for during beta, unless it is obvious.

Why should we take the trouble to break the backward
compatibility in the first place ? The only reason
that I can see is that you dislike it.

regards,
Hiroshi Inoue
        http://www.geocities.jp/inocchichichi/psqlodbc/

---------------------------(end of broadcast)---------------------------



Mon, 05 Sep 2005 12:19:17 GMT
 A bad behavior under autocommit off mode

Quote:




> > > > Does the change worth the trouble ?
> > > > Please don't break BACKWARD COMPATIBILITY easily.

> > > If it's important enough, we can provide a GUC option for it.

> > > My guess is that a GUC option isn't needed, but then again we've been
> > > criticized for disregarding backward compatibility in the past...

> > I think our policy is going to be that GUC backward compatibility
> > options have to be asked for during beta, unless it is obvious.

> Why should we take the trouble to break the backward
> compatibility in the first place ? The only reason
> that I can see is that you dislike it.

The big question is does it make sense to anyone?  I don't think so.

And if someone does, how should it behave when autocommit is off?

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------



Mon, 05 Sep 2005 12:24:06 GMT
 A bad behavior under autocommit off mode

Quote:


>> This patch removes that "stuff all queries into a single transaction"
>> behavior and makes them function just like queries arriving separately.
>> This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
>> behavior, they just need to wrap BEGIN/COMMIT around the query string.
> Does the change worth the trouble ?
> Please don't break BACKWARD COMPATIBILITY easily.

I do not like this change either.  It breaks long-established behavior
simply to have an easy fix for a recently-introduced bug (and what's
more, a bug in a feature that we may end up removing completely; I like
Peter's idea that autocommit on the client side is a better approach).

It would be a serious error to imagine that psql -c strings are the only
case where this behavior applies.  PQexec and interfaces based on it
exhibit the same behavior.  The behavior is actually useful for
pipelining (send several queries in one PQsendQuery, read and process
the results one at a time with PQgetResult; then the server's processing
of the additional commands is overlapped with client-side processing of
the results).  So I believe there are applications out there depending
on it.

I think we should look for a fix that does not break compatibility.

                        regards, tom lane

---------------------------(end of broadcast)---------------------------



Mon, 05 Sep 2005 12:31:53 GMT
 A bad behavior under autocommit off mode

Quote:

> My proposal also fixes the bug though Tom objected to it.

I've forgotten what your proposal was?

                        regards, tom lane

---------------------------(end of broadcast)---------------------------



Mon, 05 Sep 2005 12:33:27 GMT
 A bad behavior under autocommit off mode

Quote:



> >> This patch removes that "stuff all queries into a single transaction"
> >> behavior and makes them function just like queries arriving separately.
> >> This does BREAK BACKWARD COMPATIBILITY.  However, if they want the old
> >> behavior, they just need to wrap BEGIN/COMMIT around the query string.

> > Does the change worth the trouble ?
> > Please don't break BACKWARD COMPATIBILITY easily.

> I do not like this change either.  It breaks long-established behavior
> simply to have an easy fix for a recently-introduced bug (and what's
> more, a bug in a feature that we may end up removing completely; I like
> Peter's idea that autocommit on the client side is a better approach).

I don't like pushing autocommit to the client.

Quote:
> It would be a serious error to imagine that psql -c strings are the only
> case where this behavior applies.  PQexec and interfaces based on it
> exhibit the same behavior.  The behavior is actually useful for
> pipelining (send several queries in one PQsendQuery, read and process
> the results one at a time with PQgetResult; then the server's processing
> of the additional commands is overlapped with client-side processing of
> the results).  So I believe there are applications out there depending
> on it.

The fix only changes the 'make it all one transaction' behavior.  It
does not effect sending multiple queries in a string --- what will
happen with the patch is that the queries will be processed using normal
transaction commit rules, rather than bunched up.  Yes, I am sure this
bunching is used a lot.

Clearly we don't want to do this just to fix autocommit --- there are
other ways.  But I do think the roll-queries-into-one-transaction is
strange and should be removed with the patch.

--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org



Mon, 05 Sep 2005 12:36:53 GMT
 A bad behavior under autocommit off mode

Quote:

> The fix only changes the 'make it all one transaction' behavior.  It
> does not effect sending multiple queries in a string ---

Yes it does!  The results may change.  Also the behavior if later
commands in the string fail will be different (roll back earlier
commands vs not).

Quote:
> But I do think the roll-queries-into-one-transaction is
> strange and should be removed with the patch.

I disagree.  This is long-established behavior and no one has complained
about it.  We have gone out of our way to preserve it in past changes;
I don't like suddenly deciding that backwards compatibility is
unimportant.  Especially not if only one person is in favor of the change.

                        regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster



Mon, 05 Sep 2005 12:49:13 GMT
 
 [ 24 post ]  Go to page: [1] [2]

 Relevant Pages 

1. A bad behavior under autocommit off mode

2. A bad behavior under autocommit off mode

3. turning off autocommit behavior in psql

4. turning off autocommit behavior in psql

5. autocommit off mode, how does it work?

6. How to turn the autocommit off?

7. Turning Autocommit OFF?

8. Question about AutoCommit Off

9. Fix for error in autocommit off

10. ADO and autocommit off

11. Turn off autocommit?

12. Autocommit On even when it′s turned off??????


 
Powered by phpBB® Forum Software