Thread: A bad behavior under autocommit off mode
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 Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue <Inoue@tpf.co.jp> writes: > 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 Surely that's a bug: the SELECT ought to start a transaction block. Barry Lind reported what is probably a closely related issue: http://archives.postgresql.org/pgsql-hackers/2003-01/msg00592.php I haven't gotten around to looking at this, but I suspect postgres.c is doing something inside the per-querytree loop that it should be doing outside it, or vice versa. Or possibly the problem is with the klugy way that we hacked autocommit-off into the xact.c state machine. Do you have time to look at it? regards, tom lane
On Thursday 20 February 2003 10:38, Tom Lane wrote:
> Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> > 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
>
> Surely that's a bug: the SELECT ought to start a transaction block.
Sure but doesn't it also commit it? There's still no transaction open coming
out of the SELECT.
--
D'Arcy J.M. Cain <darcy@{druid|vex}.net> | Democracy is three wolves
http://www.druid.net/darcy/ | and a sheep voting on
+1 416 425 1212 (DoD#0082) (eNTP) | what's for dinner.
"D'Arcy J.M. Cain" <darcy@druid.net> writes:
> On Thursday 20 February 2003 10:38, Tom Lane wrote:
>> Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> 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
>>
>> Surely that's a bug: the SELECT ought to start a transaction block.
> Sure but doesn't it also commit it?
Not in autocommit-off mode.
regards, tom lane
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > 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 > > Surely that's a bug: the SELECT ought to start a transaction block. > > Barry Lind reported what is probably a closely related issue: > http://archives.postgresql.org/pgsql-hackers/2003-01/msg00592.php > > I haven't gotten around to looking at this, but I suspect postgres.c > is doing something inside the per-querytree loop that it should be > doing outside it, or vice versa. Or possibly the problem is with > the klugy way that we hacked autocommit-off into the xact.c state > machine. Do you have time to look at it? I have little time. The transaction block state seems to be set just before returning from the chained query. I don't know if it's bad or not. The simplest way seems to accept COMMIT any time under autocommit off mode. regards, Hiroshi Inoue
"Hiroshi Inoue" <inoue@tpf.co.jp> writes:
> The simplest way seems to accept COMMIT any time under autocommit
> off mode.
That's just hiding the most visible symptom. The real problem here is
that the SELECT is already committed, when it shouldn't be.
regards, tom lane
Tom Lane wrote: > > "Hiroshi Inoue" <inoue@tpf.co.jp> writes: > > The simplest way seems to accept COMMIT any time under autocommit > > off mode. > > That's just hiding the most visible symptom. The real problem here is > that the SELECT is already committed, when it shouldn't be. The warning means that the transaction is not yet begun before the chained query is issued. The check seems originally for COMMIT without BEGIN under autocommit on mode. It also cancels a transaction for the query '..;..;commit;..' under autocommit on mode. It's also bad because it only reports a warning. Anyway should 'set autocommit to off;commit' cause a warning or an error in the first place ? regards, Hiroshi Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> Anyway should 'set autocommit to off;commit' cause
> a warning or an error in the first place ?
IIRC, the SET does *not* start a transaction, so the COMMIT should raise
a warning.
I do not believe that eliminating the warning from COMMIT is a good
idea. If we didn't have that warning in place, we'd have not known that
we had a bug here. (On the other hand, I'm not in favor of making it
a hard error, either.)
regards, tom lane
Tom Lane wrote: > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > Anyway should 'set autocommit to off;commit' cause > > a warning or an error in the first place ? > > IIRC, the SET does *not* start a transaction, Yes but doesn't autocommit-off mode mean that it implicitly begins a transaction in suitable places ? For example, 'set autocommit to off; declare .. cursor ..' works though it never work without BEGIN under autocommit-on mode. > so the COMMIT should raise > a warning. regards, Hiroshi Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> Tom Lane wrote:
>> IIRC, the SET does *not* start a transaction,
> Yes but doesn't autocommit-off mode mean that
> it implicitly begins a transaction in suitable
> places ? For example, 'set autocommit to off;
> declare .. cursor ..' works though it never
> work without BEGIN under autocommit-on mode.
But the DECLARE would start a transaction --- AFAIR, pretty much
everything except SET, COMMIT, ROLLBACK will start a transaction
in autocommit=off mode. I'm not sure what your point is?
regards, tom lane
Tom Lane wrote: > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > Tom Lane wrote: > >> IIRC, the SET does *not* start a transaction, > > > Yes but doesn't autocommit-off mode mean that > > it implicitly begins a transaction in suitable > > places ? For example, 'set autocommit to off; > > declare .. cursor ..' works though it never > > work without BEGIN under autocommit-on mode. > > But the DECLARE would start a transaction --- AFAIR, Yes it's only because the behavior is useful for us. So isn't the problem if the warning message for 'set autocommit to off;commit' is useful or not ? IMHO it's rather a harmful message. regards, Hiroshi Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Can someone point me to the documentation for the new autocommit mode behaviour, I must be doing something wrong I do a set autocommit=off; begin; create table foo ... insert into foo end; and if I look for the table foo from another connection, it isn't there? my source tree is REL7_3_STABLE -- Dave Cramer <dave@fastcrypt.com> Cramer Consulting
Dave Cramer <dave@fastcrypt.com> writes:
> Can someone point me to the documentation for the new autocommit mode
> behaviour, I must be doing something wrong
Must be :-(. I repeated your example as best I could, and it behaved
as expected.
<< session 1 >>
regression=# set autocommit=off;
SET
regression=# begin;
BEGIN
regression=# create table foo (f1 int);
CREATE TABLE
regression=# insert into foo values (1);
INSERT 933631 1
regression=# end;
COMMIT
regression=#
<< session 2 >>
regression=# \d foo Table "public.foo"Column | Type | Modifiers
--------+---------+-----------f1 | integer |
regression=# select * from foo;f1
---- 1
(1 row)
regards, tom lane
what cvs version are you working with? Dave On Wed, 2003-02-26 at 01:11, Tom Lane wrote: > Dave Cramer <dave@fastcrypt.com> writes: > > Can someone point me to the documentation for the new autocommit mode > > behaviour, I must be doing something wrong > > Must be :-(. I repeated your example as best I could, and it behaved > as expected. > > << session 1 >> > > regression=# set autocommit=off; > SET > regression=# begin; > BEGIN > regression=# create table foo (f1 int); > CREATE TABLE > regression=# insert into foo values (1); > INSERT 933631 1 > regression=# end; > COMMIT > regression=# > > << session 2 >> > > regression=# \d foo > Table "public.foo" > Column | Type | Modifiers > --------+---------+----------- > f1 | integer | > > regression=# select * from foo; > f1 > ---- > 1 > (1 row) > > > regards, tom lane -- Dave Cramer <dave@fastcrypt.com> Cramer Consulting
Sorry for the noise, my instance of postgres was broken. Dave On Wed, 2003-02-26 at 02:19, Dave Cramer wrote: > what cvs version are you working with? > > Dave > On Wed, 2003-02-26 at 01:11, Tom Lane wrote: > > Dave Cramer <dave@fastcrypt.com> writes: > > > Can someone point me to the documentation for the new autocommit mode > > > behaviour, I must be doing something wrong > > > > Must be :-(. I repeated your example as best I could, and it behaved > > as expected. > > > > << session 1 >> > > > > regression=# set autocommit=off; > > SET > > regression=# begin; > > BEGIN > > regression=# create table foo (f1 int); > > CREATE TABLE > > regression=# insert into foo values (1); > > INSERT 933631 1 > > regression=# end; > > COMMIT > > regression=# > > > > << session 2 >> > > > > regression=# \d foo > > Table "public.foo" > > Column | Type | Modifiers > > --------+---------+----------- > > f1 | integer | > > > > regression=# select * from foo; > > f1 > > ---- > > 1 > > (1 row) > > > > > > regards, tom lane -- Dave Cramer <dave@fastcrypt.com> Cramer Consulting
Dave Cramer <dave@fastcrypt.com> writes:
> what cvs version are you working with?
CVS tip in either HEAD or REL7_3_STABLE branches works for me.
There are some known issues with autocommit getting turned off
as part of a multi-statement command that's sent to tbe backend
in a single query string. But your example in psql wouldn't have
triggered that problem.
regards, tom lane
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.
---------------------------------------------------------------------------
Hiroshi Inoue wrote:
> 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.geocities.jp/inocchichichi/psqlodbc/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>
--
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
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;
Bruce Momjian wrote: > > 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 Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue wrote: > Bruce Momjian wrote: > > > > 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 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
Bruce Momjian wrote: > > Hiroshi Inoue wrote: > > Bruce Momjian wrote: > > > > > > 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 Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue wrote: > > > > 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 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
Bruce Momjian wrote: > > Hiroshi Inoue wrote: > > > > > 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 Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue wrote: > Bruce Momjian wrote: > > > > Hiroshi Inoue wrote: > > > > > > 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 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
On Wed, 2003-03-19 at 18:55, Hiroshi Inoue wrote: > 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
Neil Conway wrote: > On Wed, 2003-03-19 at 18:55, Hiroshi Inoue wrote: > > 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 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
Bruce Momjian wrote: > > Neil Conway wrote: > > On Wed, 2003-03-19 at 18:55, Hiroshi Inoue wrote: > > > 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 Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue wrote: > Bruce Momjian wrote: > > > > Neil Conway wrote: > > > On Wed, 2003-03-19 at 18:55, Hiroshi Inoue wrote: > > > > 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 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
Hiroshi Inoue <Inoue@tpf.co.jp> writes: > Bruce Momjian wrote: >> 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
Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> My proposal also fixes the bug though Tom objected to it.
I've forgotten what your proposal was?
regards, tom lane
Tom Lane wrote: > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > Bruce Momjian wrote: > >> 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. > 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 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> 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).
> 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
On Wed, 2003-03-19 at 23:36, Bruce Momjian wrote: > I don't like pushing autocommit to the client. It seems like a good idea to me, particularly since the current server-side implementation is such a kludge. Can you go over your objections to client-side autocommit again? Cheers, Neil
OK, I am for it, and two are against. Any one else want to vote this? --------------------------------------------------------------------------- Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > 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). > > > 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 > -- 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
Neil Conway wrote: > On Wed, 2003-03-19 at 23:36, Bruce Momjian wrote: > > I don't like pushing autocommit to the client. > > It seems like a good idea to me, particularly since the current > server-side implementation is such a kludge. Can you go over your > objections to client-side autocommit again? First, I am afraid a kludge on the server is going to become 10 client kludges. I also don't see how it can be easily controlled in the application without adding an API function for every language --- adding it to libpq will require new API's in perhaps 7 different languages. I think our SET functionality is easy to understand and use. I don't see pushing it into the client as greatly improving things, and could make things worse. If we can't get it right in the backend, how many clients are going to do it wrong? -- 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
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > 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). Right, people are using it, but do they even know that those are all one transaction? I bet most don't. > > 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. I asked for some votes. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I think our SET functionality is easy to understand and use. I don't
> see pushing it into the client as greatly improving things, and could
> make things worse. If we can't get it right in the backend, how many
> clients are going to do it wrong?
This argument overlooks the fact that most of the client libraries
already have notions of autocommit on/off semantics that they need to
adhere to. libpq is too simple to have heard of the concept, but I
believe that JDBC, ODBC, and DBI/DBD all need to deal with it anyway.
I doubt that managing a server-side facility makes their lives any
easier ... especially not if its semantics don't quite match what
they need to do, which seems very possible.
But it'd be interesting to hear what the JDBC and ODBC maintainers
think about it. Perhaps autocommit as a GUC variable is just what
they want.
Please recall that GUC-autocommit in its current form was my idea,
and I rushed it in there because I wanted us to be able to run the
NIST compliance tests easily. In hindsight I am thinking it was a
bad move.
regards, tom lane
Tom, From the jdbc driver perspective I prefer the GUC variable approach, but either can be used. Each has limitations. In 7.2 and earlier jdbc code the driver handled the transaction symantics by adding begin/commit/rollback in appropriate places. And that code is still in the 7.3 driver to support older servers. In 7.3 the driver uses the GUC variable to control the transaction state. In general this is easier since it is a set once and forget about it operation. As I mentioned earlier each method has limitations. Let me list them. The problem with managing the state on the client is that in order for this to work the client needs to intercept all transaction ending events in order to start the next transaction when running in non-autocommit mode. Thus each 'commit' becomes 'commit; begin;'. Since the jdbc API has a commit() and rollback() method there is an obvious place to insert this logic. However if the user directly issues a commit or rollback sql call (instead of using the jdbc api) then the driver isn't in a position to start the new transaction, unless the driver starts parsing all SQL looking for commits or rollbacks which I am reluctant to do. However the proposed FE/BE protocol change to tell the client the transaction state would allow the driver to detect this. The problem with using the GUC approach is that if the user in SQL changed the GUC value the driver would have no way to know the state change. And thus the driver would think it was opperating in one mode (the mode *it* set), but actually be running in a different mode (the mode the *user* set through SQL). Of these two limitations the first is more significant since users do issue 'commit' statements directly sometimes, whereas users would likely never change the GUC parameter in their SQL. I like the simplicity of the GUC parameter and that is the reason I converted the jdbc driver in 7.3 to use this new method. thanks, --Barry Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>I think our SET functionality is easy to understand and use. I don't >>see pushing it into the client as greatly improving things, and could >>make things worse. If we can't get it right in the backend, how many >>clients are going to do it wrong? > > > This argument overlooks the fact that most of the client libraries > already have notions of autocommit on/off semantics that they need to > adhere to. libpq is too simple to have heard of the concept, but I > believe that JDBC, ODBC, and DBI/DBD all need to deal with it anyway. > I doubt that managing a server-side facility makes their lives any > easier ... especially not if its semantics don't quite match what > they need to do, which seems very possible. > > But it'd be interesting to hear what the JDBC and ODBC maintainers > think about it. Perhaps autocommit as a GUC variable is just what > they want. > > Please recall that GUC-autocommit in its current form was my idea, > and I rushed it in there because I wanted us to be able to run the > NIST compliance tests easily. In hindsight I am thinking it was a > bad move. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org >
On Thu, 2003-03-20 at 13:41, Barry Lind wrote: > However the proposed FE/BE protocol change to tell the client the > transaction state would allow the driver to detect this. > <snip> > > Of these two limitations the first is more significant since users do > issue 'commit' statements directly sometimes, whereas users would likely > never change the GUC parameter in their SQL. I like the simplicity of > the GUC parameter and that is the reason I converted the jdbc driver in > 7.3 to use this new method. > While the first may seem more significant, it sounds like it is actually going away with the fe/be protocol changes. If thats true, it seems to me this makes the GUC method the most limiting. Robert Treat
Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I think our SET functionality is easy to understand and use. I don't > > see pushing it into the client as greatly improving things, and could > > make things worse. If we can't get it right in the backend, how many > > clients are going to do it wrong? > > This argument overlooks the fact that most of the client libraries > already have notions of autocommit on/off semantics that they need to > adhere to. libpq is too simple to have heard of the concept, but I > believe that JDBC, ODBC, and DBI/DBD all need to deal with it anyway. > I doubt that managing a server-side facility makes their lives any > easier ... especially not if its semantics don't quite match what > they need to do, which seems very possible. > > But it'd be interesting to hear what the JDBC and ODBC maintainers > think about it. The current ODBC driver doesn't work well under autocommit off mode at server side. However, it's not on my (at least ASAP) TODO item. regards, Hiroshi Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Tom Lane wrote: > > Hiroshi Inoue <Inoue@tpf.co.jp> writes: > > My proposal also fixes the bug though Tom objected to it. > > I've forgotten what your proposal was? Because WARNING isn't an ERROR, treat it in the same way as NORMAL, i.e after the WARNING, simply set the blockState to TBLOCK_END without calling AbortTransaction. Also remove the WARNING in case of autocommit off mode. regards, Hiroshi Inouehttp://www.geocities.jp/inocchichichi/psqlodbc/
Hiroshi Inoue wrote:
> Tom Lane wrote:
> >
> > Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> > > My proposal also fixes the bug though Tom objected to it.
> >
> > I've forgotten what your proposal was?
>
> Because WARNING isn't an ERROR, treat it in the same way
> as NORMAL, i.e after the WARNING, simply set the blockState
> to TBLOCK_END without calling AbortTransaction. Also
> remove the WARNING in case of autocommit off mode.
I think our current code is working fine _except_ for the
multi-statement query case.
I lost the vote on changing the current behavior, so this patch merely
turns off this grouping when autocommit is off. That seems like the
easiest solution.
The next issue is that we don't have the current behavior documented
anywhere. I think it needs to be added to libpq's PQexec() and psql -c.
It will say:
With autocommit on, if multiple statements are sent in a single
string, all statements are grouped into a single transaction.
--
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
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.318
diff -c -c -r1.318 postgres.c
*** src/backend/tcop/postgres.c 20 Mar 2003 07:02:10 -0000 1.318
--- src/backend/tcop/postgres.c 21 Mar 2003 05:25:59 -0000
***************
*** 83,88 ****
--- 83,90 ----
bool Warn_restart_ready = false;
bool InError = false;
+ extern bool autocommit;
+
static bool EchoQuery = false; /* default don't echo */
/*
***************
*** 893,899 ****
* 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;
--- 895,901 ----
* historical Postgres behavior, we do not force a transaction
* boundary between queries appearing in a single query string.
*/
! if ((lnext(parsetree_item) == NIL || !autocommit) && xact_started)
{
finish_xact_command(false);
xact_started = false;
Barry Lind <blind@xythos.com> writes:
> Of these two limitations the first is more significant since users do
> issue 'commit' statements directly sometimes, whereas users would likely
> never change the GUC parameter in their SQL.
Well, we could fix either or both of these in the planned protocol
change. It would be reasonable for the Z message to convey not only
the current transaction state (not in xact/in xact/in failed xact,
I believe are the three interesting states) but also the current
autocommit boolean. So assuming that visibility of state is not the
determining issue, which way do you actually like?
regards, tom lane
I have an idea. It would be nice if you could ask the backend to report the current autocommit value at the end of every query. Tom, did you have any thought of adding the ability to ask for reports on GUC variables on every query return? Barry, that would solve your problem, and I bet a few others. In fact, we could add a SHOW tranaction_status readonly GUC variable, and use that to return the transaction status for psql! --------------------------------------------------------------------------- Barry Lind wrote: > Tom, > > From the jdbc driver perspective I prefer the GUC variable approach, > but either can be used. Each has limitations. > > In 7.2 and earlier jdbc code the driver handled the transaction > symantics by adding begin/commit/rollback in appropriate places. And > that code is still in the 7.3 driver to support older servers. > > In 7.3 the driver uses the GUC variable to control the transaction > state. In general this is easier since it is a set once and forget > about it operation. > > As I mentioned earlier each method has limitations. Let me list them. > > The problem with managing the state on the client is that in order for > this to work the client needs to intercept all transaction ending events > in order to start the next transaction when running in non-autocommit > mode. Thus each 'commit' becomes 'commit; begin;'. Since the jdbc API > has a commit() and rollback() method there is an obvious place to insert > this logic. However if the user directly issues a commit or rollback > sql call (instead of using the jdbc api) then the driver isn't in a > position to start the new transaction, unless the driver starts parsing > all SQL looking for commits or rollbacks which I am reluctant to do. > However the proposed FE/BE protocol change to tell the client the > transaction state would allow the driver to detect this. > > The problem with using the GUC approach is that if the user in SQL > changed the GUC value the driver would have no way to know the state > change. And thus the driver would think it was opperating in one mode > (the mode *it* set), but actually be running in a different mode (the > mode the *user* set through SQL). > > Of these two limitations the first is more significant since users do > issue 'commit' statements directly sometimes, whereas users would likely > never change the GUC parameter in their SQL. I like the simplicity of > the GUC parameter and that is the reason I converted the jdbc driver in > 7.3 to use this new method. > > thanks, > --Barry > > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > >>I think our SET functionality is easy to understand and use. I don't > >>see pushing it into the client as greatly improving things, and could > >>make things worse. If we can't get it right in the backend, how many > >>clients are going to do it wrong? > > > > > > This argument overlooks the fact that most of the client libraries > > already have notions of autocommit on/off semantics that they need to > > adhere to. libpq is too simple to have heard of the concept, but I > > believe that JDBC, ODBC, and DBI/DBD all need to deal with it anyway. > > I doubt that managing a server-side facility makes their lives any > > easier ... especially not if its semantics don't quite match what > > they need to do, which seems very possible. > > > > But it'd be interesting to hear what the JDBC and ODBC maintainers > > think about it. Perhaps autocommit as a GUC variable is just what > > they want. > > > > Please recall that GUC-autocommit in its current form was my idea, > > and I rushed it in there because I wanted us to be able to run the > > NIST compliance tests easily. In hindsight I am thinking it was a > > bad move. > > > > regards, tom lane > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, did you have any thought of adding the ability to ask for reports
> on GUC variables on every query return?
That seems excessive. There is a case for reporting autocommit (if
we don't decide to remove it altogether, which is still an open
argument). I have not heard any complaints suggesting that we need any
others.
A fixed commitment to report xact status will cost us one byte added to
Z messages. If we add a commitment to report autocommit status, we
could choose to pack that bit into the same byte as xact status (still
only three bits used...). A slightly more forward-looking approach
would be to decree that Z messages carry labeled status bytes, ie, pairs
of <label> <status> bytes, which makes total cost 4 bytes to transmit
xact state and autocommit state. But if we want to say that we'll
transmit *any* darn random GUC variable you want to hear about, I don't
think we can use a compact encoding of this sort; instead we're talking
about sending the whole GUC variable name and string value as label and
value. In other words the Z message starts to look likeZ X a c t s t a t u s \0 i d l e \0 a u t o c o m m i t \0 o f
f\0
and somewhere around here my concern for connection bandwidth kicks in.
A 10X increase in bytes sent is a bit much to cater to hypothetical
needs.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom, did you have any thought of adding the ability to ask for reports > > on GUC variables on every query return? > > That seems excessive. There is a case for reporting autocommit (if > we don't decide to remove it altogether, which is still an open > argument). I have not heard any complaints suggesting that we need any > others. Well, the jdbc guys like SET, and I haven't seen any proposal that explains how applications would control a client-based autocommit API from the client. > A fixed commitment to report xact status will cost us one byte added to > Z messages. If we add a commitment to report autocommit status, we > could choose to pack that bit into the same byte as xact status (still > only three bits used...). A slightly more forward-looking approach > would be to decree that Z messages carry labeled status bytes, ie, pairs > of <label> <status> bytes, which makes total cost 4 bytes to transmit > xact state and autocommit state. But if we want to say that we'll > transmit *any* darn random GUC variable you want to hear about, I don't > think we can use a compact encoding of this sort; instead we're talking > about sending the whole GUC variable name and string value as label and > value. In other words the Z message starts to look like > Z X a c t s t a t u s \0 i d l e \0 a u t o c o m m i t \0 o f f \0 > and somewhere around here my concern for connection bandwidth kicks in. > A 10X increase in bytes sent is a bit much to cater to hypothetical > needs. Very true. The only other environment variable I have heard about was client encoding. As I remember right now, we do have a problem with SET changing the client encoding, and the client not realizing that. My idea may be overdesign. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Well, the jdbc guys like SET, and I haven't seen any proposal that
> explains how applications would control a client-based autocommit API
> from the client.
libpq is the only client library that doesn't already *have* an API spec
for this. SET is not helpful for any of the rest because it is not the
spec they need to meet.
> Very true. The only other environment variable I have heard about was
> client encoding. As I remember right now, we do have a problem with SET
> changing the client encoding, and the client not realizing that.
Hm. Is anyone else very concerned about that? The design roadmap I put
forward last week suggested reporting client encoding and autocommit
status during the initial connection handshake, but not after every
query. Neither of those seem like things that are sensible to change
on-the-fly during a session.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Well, the jdbc guys like SET, and I haven't seen any proposal that
> > explains how applications would control a client-based autocommit API
> > from the client.
>
> libpq is the only client library that doesn't already *have* an API spec
> for this. SET is not helpful for any of the rest because it is not the
> spec they need to meet.
True, but we have 7+ interfaces based on libpq.
> > Very true. The only other environment variable I have heard about was
> > client encoding. As I remember right now, we do have a problem with SET
> > changing the client encoding, and the client not realizing that.
>
> Hm. Is anyone else very concerned about that? The design roadmap I put
> forward last week suggested reporting client encoding and autocommit
> status during the initial connection handshake, but not after every
> query. Neither of those seem like things that are sensible to change
> on-the-fly during a session.
Well, we do have this problem mentioned in the psql \encoding manual
page:
This command will not notice changes made directly by <command>SET CLIENT_ENCODING</>. If you use
<literal>\encoding</literal>, be sure to use it to set as well as examine the encoding.
Not sure if it is worth fixing, but I thought I should mention it,
especially if people can think of other problem cases.
-- 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
Tom Lane writes:
> That seems excessive. There is a case for reporting autocommit (if
> we don't decide to remove it altogether, which is still an open
> argument). I have not heard any complaints suggesting that we need any
> others.
If we don't remove autocommit altogether on the server-side, then this
whole exercise is nearly pointless, because it still won't allow a
noninteractive libpq application to go in, do PQexec("command") and quit.
Each such application would still have to either set the autocommit state
explicitly first or, the new proposal, query the autocommit status and
then issue a COMMIT or not, as needed.
--
Peter Eisentraut peter_e@gmx.net
Currently the four variables that the jdbc driver would like to know about are (However pending other changes in 7.4, some or all of these may no longer be necessary): trx state autocommit client_encoding datestyle In the previous email I discusses why autocommit perhaps shouldn't be a GUC parameter. From a jdbc perspective what was said in that email generally applies to client_encoding and datestyle as well. The client protocol (jdbc in this case) determines how it wants to communicate with the server. It isn't really the system administrator or an end user SET that should determine that. In the jdbc case if the user were to issue a SET to change the client_encoding bad things would happen since the driver would no longer know how to correctly encode/decode information to/from the server. Likewise if the datestyle where changed, the driver wouldn't know how to parse the dates coming over the FE/BE protocol in order to create native java Date objects. Now how important is this problem? Not very IMHO. I raise it only for discussion. Over the last three years I have been involved with the postgres jdbc driver, I can't remember anyone ever complaining that they issued a SET and then the driver didn't work anymore. That is probably because in general it doesn't make sense for the end user to be changing these settings, even though the GUC funcionality allows them to do that. The setting of datestyle impacts the over the wire format for dates. In general I think it is a bad idea for the FE/BE protocol to be impacted by user and administrator decisions. Just because you want dates in psql to show one way, doesn't mean that all client protocols should have to deal with that. If you look at datestyle, autocommit and probably client_encoding as well, they provide useful functionality to users of psql, but cause work for everyone else. They probably should have been implemented as features of psql (with backend support where needed) instead of impacting the overall FE/BE protocol. thanks, --Barry Bruce Momjian wrote: > Tom Lane wrote: > >>Bruce Momjian <pgman@candle.pha.pa.us> writes: >> >>>Tom, did you have any thought of adding the ability to ask for reports >>>on GUC variables on every query return? >> >>That seems excessive. There is a case for reporting autocommit (if >>we don't decide to remove it altogether, which is still an open >>argument). I have not heard any complaints suggesting that we need any >>others. > > > Well, the jdbc guys like SET, and I haven't seen any proposal that > explains how applications would control a client-based autocommit API > from the client. > > >>A fixed commitment to report xact status will cost us one byte added to >>Z messages. If we add a commitment to report autocommit status, we >>could choose to pack that bit into the same byte as xact status (still >>only three bits used...). A slightly more forward-looking approach >>would be to decree that Z messages carry labeled status bytes, ie, pairs >>of <label> <status> bytes, which makes total cost 4 bytes to transmit >>xact state and autocommit state. But if we want to say that we'll >>transmit *any* darn random GUC variable you want to hear about, I don't >>think we can use a compact encoding of this sort; instead we're talking >>about sending the whole GUC variable name and string value as label and >>value. In other words the Z message starts to look like >> Z X a c t s t a t u s \0 i d l e \0 a u t o c o m m i t \0 o f f \0 >>and somewhere around here my concern for connection bandwidth kicks in. >>A 10X increase in bytes sent is a bit much to cater to hypothetical >>needs. > > > Very true. The only other environment variable I have heard about was > client encoding. As I remember right now, we do have a problem with SET > changing the client encoding, and the client not realizing that. > > My idea may be overdesign. >
I wonder if we should allow the client to specify settings on connection start that _can't_ be changed via SET. --------------------------------------------------------------------------- Barry Lind wrote: > Currently the four variables that the jdbc driver would like to know > about are (However pending other changes in 7.4, some or all of these > may no longer be necessary): > > trx state > autocommit > client_encoding > datestyle > > In the previous email I discusses why autocommit perhaps shouldn't be a > GUC parameter. From a jdbc perspective what was said in that email > generally applies to client_encoding and datestyle as well. The client > protocol (jdbc in this case) determines how it wants to communicate with > the server. It isn't really the system administrator or an end user SET > that should determine that. In the jdbc case if the user were to issue > a SET to change the client_encoding bad things would happen since the > driver would no longer know how to correctly encode/decode information > to/from the server. Likewise if the datestyle where changed, the driver > wouldn't know how to parse the dates coming over the FE/BE protocol in > order to create native java Date objects. > > Now how important is this problem? Not very IMHO. I raise it only for > discussion. Over the last three years I have been involved with the > postgres jdbc driver, I can't remember anyone ever complaining that they > issued a SET and then the driver didn't work anymore. That is probably > because in general it doesn't make sense for the end user to be changing > these settings, even though the GUC funcionality allows them to do that. > > The setting of datestyle impacts the over the wire format for dates. In > general I think it is a bad idea for the FE/BE protocol to be impacted > by user and administrator decisions. Just because you want dates in > psql to show one way, doesn't mean that all client protocols should have > to deal with that. > > If you look at datestyle, autocommit and probably client_encoding as > well, they provide useful functionality to users of psql, but cause work > for everyone else. They probably should have been implemented as > features of psql (with backend support where needed) instead of > impacting the overall FE/BE protocol. > > thanks, > --Barry > > > > Bruce Momjian wrote: > > Tom Lane wrote: > > > >>Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> > >>>Tom, did you have any thought of adding the ability to ask for reports > >>>on GUC variables on every query return? > >> > >>That seems excessive. There is a case for reporting autocommit (if > >>we don't decide to remove it altogether, which is still an open > >>argument). I have not heard any complaints suggesting that we need any > >>others. > > > > > > Well, the jdbc guys like SET, and I haven't seen any proposal that > > explains how applications would control a client-based autocommit API > > from the client. > > > > > >>A fixed commitment to report xact status will cost us one byte added to > >>Z messages. If we add a commitment to report autocommit status, we > >>could choose to pack that bit into the same byte as xact status (still > >>only three bits used...). A slightly more forward-looking approach > >>would be to decree that Z messages carry labeled status bytes, ie, pairs > >>of <label> <status> bytes, which makes total cost 4 bytes to transmit > >>xact state and autocommit state. But if we want to say that we'll > >>transmit *any* darn random GUC variable you want to hear about, I don't > >>think we can use a compact encoding of this sort; instead we're talking > >>about sending the whole GUC variable name and string value as label and > >>value. In other words the Z message starts to look like > >> Z X a c t s t a t u s \0 i d l e \0 a u t o c o m m i t \0 o f f \0 > >>and somewhere around here my concern for connection bandwidth kicks in. > >>A 10X increase in bytes sent is a bit much to cater to hypothetical > >>needs. > > > > > > Very true. The only other environment variable I have heard about was > > client encoding. As I remember right now, we do have a problem with SET > > changing the client encoding, and the client not realizing that. > > > > My idea may be overdesign. > > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- 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
Barry Lind <blind@xythos.com> writes:
> In the previous email I discusses why autocommit perhaps shouldn't be a
> GUC parameter. From a jdbc perspective what was said in that email
> generally applies to client_encoding and datestyle as well.
Hmm. I'm not sure that it makes sense for clients to want to change
client_encoding on the fly, but it's harder to make that claim for
datestyle. And datestyle has been a SET variable for so long that we
would get a *lot* of compatibility flak if we took it away.
I had originally been thinking of reporting client_encoding as a field
of some message sent only at backend startup. However, what if we send
such a message whenever one of the variables it includes changes?
(This is basically Bruce's idea from last night, but refined to only
send the data when it changes, rather than on every query; that should
eliminate the bandwidth-wastage objection.)
I'm not convinced that it's worth the trouble to make the set of
reported variables be configurable on-the-fly as Bruce was suggesting.
client_encoding and datestyle are candidates to send this way, but are
there really others?
A point that would have to be considered is what to do when a SET
operation is rolled back by transaction abort. I think what we'd need
to do is retransmit each time the effective value changes; so it'd
work like this:
-- initial DateStyle is fooBEGIN;SET DateStyle TO bar;-- backend sends status message showing DateStyle = barSELECT
...--reported dates will use DateStyle barROLLBACK;-- backend sends status message showing DateStyle = foo
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I wonder if we should allow the client to specify settings on connection
> start that _can't_ be changed via SET.
The GUC code already has that concept (PGC_BACKEND) --- it'd be simple
to move some existing variables into that category if we decide that's
what we want to do. But I don't think we want to do so for DateStyle,
whatever position we take on autocommit.
regards, tom lane
Tom Lane wrote: > Barry Lind <blind@xythos.com> writes: > >>In the previous email I discusses why autocommit perhaps shouldn't be a >>GUC parameter. From a jdbc perspective what was said in that email >>generally applies to client_encoding and datestyle as well. > > > Hmm. I'm not sure that it makes sense for clients to want to change > client_encoding on the fly, but it's harder to make that claim for > datestyle. And datestyle has been a SET variable for so long that we > would get a *lot* of compatibility flak if we took it away. > The issues I was raising are more theoretical than practical. The only reason I was raising them, is that while we are looking at changes to the FE/BE protocol we should look at all the issues. Good general purpose mechanisms (like what Tom is suggesting below) can be created when a better understanding of all the issues is known. --Barry > I had originally been thinking of reporting client_encoding as a field > of some message sent only at backend startup. However, what if we send > such a message whenever one of the variables it includes changes? > (This is basically Bruce's idea from last night, but refined to only > send the data when it changes, rather than on every query; that should > eliminate the bandwidth-wastage objection.) > > I'm not convinced that it's worth the trouble to make the set of > reported variables be configurable on-the-fly as Bruce was suggesting. > client_encoding and datestyle are candidates to send this way, but are > there really others? > > A point that would have to be considered is what to do when a SET > operation is rolled back by transaction abort. I think what we'd need > to do is retransmit each time the effective value changes; so it'd > work like this: > > -- initial DateStyle is foo > BEGIN; > SET DateStyle TO bar; > -- backend sends status message showing DateStyle = bar > SELECT ... > -- reported dates will use DateStyle bar > ROLLBACK; > -- backend sends status message showing DateStyle = foo > > 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 >
Actually, looking at this email, having a SET that is unchangeable would fix the issues with autocommit, datestyle, and client encoding for jdbc. The basic issue is that certain interfaces want to set these parameters as unchangeable, and that would allow this. In fact, I can see value in a permanent SET ability in other cases --- I can imagine administrators wanting SET log_statement to 'on', either in postgresql.conf or per-user/db that _can't_ be changed by the user. We could specify permanent in postgresql.conf via a special syntax, like ':='. --------------------------------------------------------------------------- Barry Lind wrote: > Currently the four variables that the jdbc driver would like to know > about are (However pending other changes in 7.4, some or all of these > may no longer be necessary): > > trx state > autocommit > client_encoding > datestyle > > In the previous email I discusses why autocommit perhaps shouldn't be a > GUC parameter. From a jdbc perspective what was said in that email > generally applies to client_encoding and datestyle as well. The client > protocol (jdbc in this case) determines how it wants to communicate with > the server. It isn't really the system administrator or an end user SET > that should determine that. In the jdbc case if the user were to issue > a SET to change the client_encoding bad things would happen since the > driver would no longer know how to correctly encode/decode information > to/from the server. Likewise if the datestyle where changed, the driver > wouldn't know how to parse the dates coming over the FE/BE protocol in > order to create native java Date objects. > > Now how important is this problem? Not very IMHO. I raise it only for > discussion. Over the last three years I have been involved with the > postgres jdbc driver, I can't remember anyone ever complaining that they > issued a SET and then the driver didn't work anymore. That is probably > because in general it doesn't make sense for the end user to be changing > these settings, even though the GUC funcionality allows them to do that. > > The setting of datestyle impacts the over the wire format for dates. In > general I think it is a bad idea for the FE/BE protocol to be impacted > by user and administrator decisions. Just because you want dates in > psql to show one way, doesn't mean that all client protocols should have > to deal with that. > > If you look at datestyle, autocommit and probably client_encoding as > well, they provide useful functionality to users of psql, but cause work > for everyone else. They probably should have been implemented as > features of psql (with backend support where needed) instead of > impacting the overall FE/BE protocol. > > thanks, > --Barry > > > > Bruce Momjian wrote: > > Tom Lane wrote: > > > >>Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> > >>>Tom, did you have any thought of adding the ability to ask for reports > >>>on GUC variables on every query return? > >> > >>That seems excessive. There is a case for reporting autocommit (if > >>we don't decide to remove it altogether, which is still an open > >>argument). I have not heard any complaints suggesting that we need any > >>others. > > > > > > Well, the jdbc guys like SET, and I haven't seen any proposal that > > explains how applications would control a client-based autocommit API > > from the client. > > > > > >>A fixed commitment to report xact status will cost us one byte added to > >>Z messages. If we add a commitment to report autocommit status, we > >>could choose to pack that bit into the same byte as xact status (still > >>only three bits used...). A slightly more forward-looking approach > >>would be to decree that Z messages carry labeled status bytes, ie, pairs > >>of <label> <status> bytes, which makes total cost 4 bytes to transmit > >>xact state and autocommit state. But if we want to say that we'll > >>transmit *any* darn random GUC variable you want to hear about, I don't > >>think we can use a compact encoding of this sort; instead we're talking > >>about sending the whole GUC variable name and string value as label and > >>value. In other words the Z message starts to look like > >> Z X a c t s t a t u s \0 i d l e \0 a u t o c o m m i t \0 o f f \0 > >>and somewhere around here my concern for connection bandwidth kicks in. > >>A 10X increase in bytes sent is a bit much to cater to hypothetical > >>needs. > > > > > > Very true. The only other environment variable I have heard about was > > client encoding. As I remember right now, we do have a problem with SET > > changing the client encoding, and the client not realizing that. > > > > My idea may be overdesign. > > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Actually, looking at this email, having a SET that is unchangeable would
> fix the issues with autocommit, datestyle, and client encoding for jdbc.
> The basic issue is that certain interfaces want to set these parameters
> as unchangeable, and that would allow this.
I'm not sure they need these parameters to be *unchangeable*. What they
need is to *know what they are*, with certainty. The notion of issuing
an automatic report message whenever the values change would seem to
answer that.
On the other hand, that only directly solves the problem for a single
layer of client library. Imagine, say, a middleware layer built on top
of JDBC. If that layer wants to track the state of DATESTYLE, or any
other parameter, do these mechanisms help it? No, unless JDBC exposes
the parameter-update-reporting protocol ... which it probably won't
because that's not part of the JDBC API spec.
When you look at the problem in terms of N levels of client-side code,
I fear that none of the ideas we've discussed really provide a
satisfactory answer. Reporting doesn't work unless the reports
propagate all the way up the client stack. "Unchangeable params" don't
work --- which level gets to dictate the actual setting, and how do the
other levels learn what it is?
Any thoughts about it?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Actually, looking at this email, having a SET that is unchangeable would > > fix the issues with autocommit, datestyle, and client encoding for jdbc. > > > The basic issue is that certain interfaces want to set these parameters > > as unchangeable, and that would allow this. > > I'm not sure they need these parameters to be *unchangeable*. What they > need is to *know what they are*, with certainty. The notion of issuing > an automatic report message whenever the values change would seem to > answer that. What concerns me also about the reporting problem is that some of these interfaces must issue queries in several places in the code, so somehow they have to make sure they check for those _special_ values in all those places. > On the other hand, that only directly solves the problem for a single > layer of client library. Imagine, say, a middleware layer built on top > of JDBC. If that layer wants to track the state of DATESTYLE, or any > other parameter, do these mechanisms help it? No, unless JDBC exposes > the parameter-update-reporting protocol ... which it probably won't > because that's not part of the JDBC API spec. Good point. > When you look at the problem in terms of N levels of client-side code, > I fear that none of the ideas we've discussed really provide a > satisfactory answer. Reporting doesn't work unless the reports > propagate all the way up the client stack. "Unchangeable params" don't > work --- which level gets to dictate the actual setting, and how do the > other levels learn what it is? > > Any thoughts about it? What it seems we need is some way to know that a SET is coming from the interface vs. coming from the user, and as you point out, that may not be completely clear. I am also hoping to find something that is generic, not tied to a specific SET variable, because I assume this will come up in the future. One idea I had was to have a SET INTERFACE option, that could only be changed by another SET INTERFACE option. It wouldn't prevent a client from doing it, but it would make it pretty clear that the user was messing with something the interface wants to control. A more bizarre idea is that SET INTERFACE would have a password that could only be changed by another SET INTERFACE with the same password! Also, with autocommit, is the idea for autocommit logic to be in the clients, or just control of autocommit to be in the clients, with the logic still being in the server? -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> I'm not sure they need these parameters to be *unchangeable*. What they
>> need is to *know what they are*, with certainty. The notion of issuing
>> an automatic report message whenever the values change would seem to
>> answer that.
> What concerns me also about the reporting problem is that some of these
> interfaces must issue queries in several places in the code, so somehow
> they have to make sure they check for those _special_ values in all
> those places.
Not sure what your point is here. If an interface is going to support
more than one value of a parameter, then yes it has to be sure to do the
right thing in each affected place. There's no shortcut for that.
> Also, with autocommit, is the idea for autocommit logic to be in the
> clients, or just control of autocommit to be in the clients, with the
> logic still being in the server?
My thought is to put it in the clients. All except libpq already have
some such logic, and it worked well enough except for their inability to
be completely sure of the backend's state --- which we will fix with the
protocol revision. The server-side implementation has turned out to be
a complete mess.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> I'm not sure they need these parameters to be *unchangeable*. What they > >> need is to *know what they are*, with certainty. The notion of issuing > >> an automatic report message whenever the values change would seem to > >> answer that. > > > What concerns me also about the reporting problem is that some of these > > interfaces must issue queries in several places in the code, so somehow > > they have to make sure they check for those _special_ values in all > > those places. > > Not sure what your point is here. If an interface is going to support > more than one value of a parameter, then yes it has to be sure to do the > right thing in each affected place. There's no shortcut for that. I realize the transaction status varies from query to query, so that can be hard-wired into the protocol --- but the other ones seem mostly to be cases where you don't want the user changing something behind the back of the interface. If we give the interface more control, we don't have to report back the SET status. The only case that doesn't work is for psql client encoding, where psql wants to know the user changed it --- in such cases a SET in psql would fail, and the user would be instructed to use the interface-specific method for changing that variable. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I realize the transaction status varies from query to query, so that can
> be hard-wired into the protocol --- but the other ones seem mostly to be
> cases where you don't want the user changing something behind the back
> of the interface.
Right.
> If we give the interface more control, we don't have
> to report back the SET status.
That seems to be going in the wrong direction, though, in terms of
flexibility.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I realize the transaction status varies from query to query, so that can > > be hard-wired into the protocol --- but the other ones seem mostly to be > > cases where you don't want the user changing something behind the back > > of the interface. > > Right. > > > If we give the interface more control, we don't have > > to report back the SET status. > > That seems to be going in the wrong direction, though, in terms of > flexibility. I was just trying to avoid the interface complexity _if_ preventing SET was all that was needed. -- 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
Tom Lane writes: > I had originally been thinking of reporting client_encoding as a field > of some message sent only at backend startup. However, what if we send > such a message whenever one of the variables it includes changes? The silent assumption behind the client_encoding parameter is that you must set it to the actual character set encoding used by the client. If you lie, the results are unspecified. So if you're in a JDBC application and set the client encoding to an encoding that the JDBC driver (that is, "the client") cannot handle, you lied and you deserve to lose. (Really, this problem can only occur in applications that let random users enter random commands or if a programmer is explicitly trying out forbidden territory.) There are real and valid reasons for changing the client encoding on the fly, but that is no reason to make a big deal about passing the information around all the time. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> The silent assumption behind the client_encoding parameter is that you
> must set it to the actual character set encoding used by the client. If
> you lie, the results are unspecified. So if you're in a JDBC application
> and set the client encoding to an encoding that the JDBC driver (that is,
> "the client") cannot handle, you lied and you deserve to lose.
I think the issue is not so much whether the JDBC driver *can* handle
the encoding, as whether it knows what it needs to be doing right now.
> There are real and valid reasons for changing the client encoding on the
> fly, but that is no reason to make a big deal about passing the
> information around all the time.
If the JDBC driver needs to do anything different for one encoding than
another, then it needs to be informed of changes. We can debate what's
the most appropriate way to keep it informed, but I don't think we can
just ignore the need to inform it.
regards, tom lane
Tom Lane writes: > If the JDBC driver needs to do anything different for one encoding than > another, then it needs to be informed of changes. We can debate what's > the most appropriate way to keep it informed, but I don't think we can > just ignore the need to inform it. We can just say, "Don't alter the client encoding behind the back of the driver." -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> We can just say, "Don't alter the client encoding behind the back of the
> driver."
We can perhaps get away with saying that for client_encoding, but what
of DateStyle? "SET" has been the traditional way to adjust that since
the stone age.
It seems to me there is not a lot of distance between what I originally
suggested (transmit values of interesting variables at connection start)
and what we're talking about here (transmit values of interesting
variables at connection start and then again if they change). I'm more
than willing to do the small amount of additional work needed, if it
makes interface libraries' job easier.
regards, tom lane
OK, I have applied the following patch to fix the original bug report:
psql -c "SET autocommit TO off;SELECT 1;COMMIT;" template1
It turns off grouping of queries into a single transaction when
autocommit is off, and documents that grouping behavior when autocommit
is on. (It also removes a mention of porting from 6.4.)
This may get removed if we move autocommit to the client, but at least
it is done, and documented.
---------------------------------------------------------------------------
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > 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).
>
> Right, people are using it, but do they even know that those are all one
> transaction? I bet most don't.
>
> > > 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.
>
> I asked for some votes.
>
> --
> 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 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>
--
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
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/libpq.sgml,v
retrieving revision 1.114
diff -c -c -r1.114 libpq.sgml
*** doc/src/sgml/libpq.sgml 22 Mar 2003 03:29:05 -0000 1.114
--- doc/src/sgml/libpq.sgml 24 Mar 2003 18:21:08 -0000
***************
*** 857,867 ****
maintain the <structname>PGresult</structname> abstraction. Use the accessor functions below to get
at the contents of <structname>PGresult</structname>. Avoid directly referencing the fields of the
<structname>PGresult</structname> structure because they are subject to change in the future.
! (Beginning in <productname>PostgreSQL</productname> 6.4, the
! definition of <type>struct</> behind <structname>PGresult</> is not even provided in <filename>libpq-fe.h</>. If you
! have old code that accesses <structname>PGresult</structname> fields directly, you can keep using it
! by including <filename>libpq-int.h</filename> too, but you are encouraged to fix the code
! soon.)
</para>
<variablelist>
--- 857,864 ----
maintain the <structname>PGresult</structname> abstraction. Use the accessor functions below to get
at the contents of <structname>PGresult</structname>. Avoid directly referencing the fields of the
<structname>PGresult</structname> structure because they are subject to change in the future.
! If <quote>autocommit</quote> is on, multiple queries sent in a single
! function call are processed in a single transaction.
</para>
<variablelist>
Index: doc/src/sgml/ref/psql-ref.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.85
diff -c -c -r1.85 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml 13 Feb 2003 05:37:43 -0000 1.85
--- doc/src/sgml/ref/psql-ref.sgml 24 Mar 2003 18:21:11 -0000
***************
*** 87,92 ****
--- 87,97 ----
<application>psql</application>, like this: <literal>echo "\x \\
select * from foo;" | psql</literal>.
</para>
+ <para>
+ If <quote>autocommit</quote> is on, multiple queries in a single
+ string are processed in a single transaction.
+
+ </para>
</listitem>
</varlistentry>
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.319
diff -c -c -r1.319 postgres.c
*** src/backend/tcop/postgres.c 22 Mar 2003 04:23:34 -0000 1.319
--- src/backend/tcop/postgres.c 24 Mar 2003 18:21:16 -0000
***************
*** 83,88 ****
--- 83,90 ----
bool Warn_restart_ready = false;
bool InError = false;
+ extern bool autocommit;
+
static bool EchoQuery = false; /* default don't echo */
/*
***************
*** 893,899 ****
* 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;
--- 895,901 ----
* historical Postgres behavior, we do not force a transaction
* boundary between queries appearing in a single query string.
*/
! if ((lnext(parsetree_item) == NIL || !autocommit) && xact_started)
{
finish_xact_command(false);
xact_started = false;
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > We can just say, "Don't alter the client encoding behind the back of the > > driver." > > We can perhaps get away with saying that for client_encoding, but what > of DateStyle? "SET" has been the traditional way to adjust that since > the stone age. > > It seems to me there is not a lot of distance between what I originally > suggested (transmit values of interesting variables at connection start) > and what we're talking about here (transmit values of interesting > variables at connection start and then again if they change). I'm more > than willing to do the small amount of additional work needed, if it > makes interface libraries' job easier. I have been thinking about this SET thing, and here are some more ideas. One idea is for SET to return a command tag that has more information, like we do for INSERT/UPDATE/DELETE. It could return the variable modified and the new value. Of course, that doesn't help with SET inside transactions because it is only the COMMIT where it should show. One idea there is for COMMIT to indicate that _some_ SET variable was changed, and the client has to query the backend for each value it is in interested in. There is also RESET to deal with, and RESET ALL, which could change multiple SET values. For autocommit on the client side, I assume folks realize that the clients are going to have to parse the query string to find out if BEGIN is needed, e.g.: INSERT;COMMIT;INSERT;COMMIT It will have to deal with quote detection and backslash handling, e.g. \; doesn't terminate a query, I think. Also, are we removing the behavior that SET _doesn't_ start a transaction in autocommit off mode? As I remember, autocommit was the primary reason we wanted that behavior, and with no SET AUTOCOMMIT, it seems strange. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> One idea is for SET to return a command tag that has more information,
> like we do for INSERT/UPDATE/DELETE. It could return the variable
> modified and the new value.
But that doesn't solve the problem --- what about begin, set, rollback?
What about absorbing a new value for a variable while re-reading
postgresql.conf due to SIGHUP?
Unless you want to effectively disable all of the nice GUC behavior
we've developed, I think you have to have a reporting mechanism that's
separate from command completion.
> Also, are we removing the behavior that SET _doesn't_ start a
> transaction in autocommit off mode?
If we remove autocommit-off mode, it stops being an issue ;-)
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > One idea is for SET to return a command tag that has more information, > > like we do for INSERT/UPDATE/DELETE. It could return the variable > > modified and the new value. > > But that doesn't solve the problem --- what about begin, set, rollback? > What about absorbing a new value for a variable while re-reading > postgresql.conf due to SIGHUP? > > Unless you want to effectively disable all of the nice GUC behavior > we've developed, I think you have to have a reporting mechanism that's > separate from command completion. Yes, rereading the config file would kill my idea --- but what API are we going to pass SET to applications? I can't think of a clean method, yet. > > Also, are we removing the behavior that SET _doesn't_ start a > > transaction in autocommit off mode? > > If we remove autocommit-off mode, it stops being an issue ;-) Sure, but how are we going to treat SET in the client? -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Yes, rereading the config file would kill my idea --- but what API are
> we going to pass SET to applications?
Passing the info up the client-side stack is an issue, yes, but it will
be so in any case. If it's not there in the protocol we haven't even
got a foothold to solve the problem ...
> Sure, but how are we going to treat SET in the client?
Not following your concern here. SET is what it always was.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Yes, rereading the config file would kill my idea --- but what API are > > we going to pass SET to applications? > > Passing the info up the client-side stack is an issue, yes, but it will > be so in any case. If it's not there in the protocol we haven't even > got a foothold to solve the problem ... > > > Sure, but how are we going to treat SET in the client? > > Not following your concern here. SET is what it always was. The question is whether a client-side implementation of autocommit is going to allow SET to being a transaction when autocommit is off. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The question is whether a client-side implementation of autocommit is
> going to allow SET to begin a transaction when autocommit is off.
Well, that'd be up to the client to decide ... but I would imagine
they'd probably make it do so. AFAIR the reason we wanted SET not to
start a transaction was only for AUTOCOMMIT, and there's no reason
to special-case it otherwise.
regards, tom lane
Tom Lane writes: > We can perhaps get away with saying that for client_encoding, but what > of DateStyle? "SET" has been the traditional way to adjust that since > the stone age. The JDBC driver sets the datestyle on startup and there's no reason why a client application would explicitly want to change it to defeat the JDBC driver. So "don't do that" applies here as well. It could be helpful to have a command to set a value and not allow it to be changed afterwards. But are there *real* applications where it would make a difference? And where does it stop? There are about a dozen GUC variables that will break an application as a whole if they don't have the value expected by the application. Do we need to install guards against all of these? > It seems to me there is not a lot of distance between what I originally > suggested (transmit values of interesting variables at connection start) > and what we're talking about here (transmit values of interesting > variables at connection start and then again if they change). I thought the originally proposed transmission was from the client to the server (client encoding, time zone) whereas this transmission would be from the server to the client. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> And where does it stop? There are about a dozen GUC variables that will
> break an application as a whole if they don't have the value expected by
> the application. Do we need to install guards against all of these?
The issue in my mind is not what will break an application, but what
will break a client-side library. The application knows, in some sense,
what settings it has selected -- either because it did explicit SETs or
because it's coded expecting certain values to be supplied via the GUC
default mechanisms. And the server knows what values have been set,
too. But the client-side library is out of the loop. We need to bring
it into the loop, at least for the values it needs to know about (and
yes, I agree that that's not a very well-defined set, but we can easily
set up the protocol to allow an expansible set of variables to be
transmitted).
I think that "don't do that" is not an acceptably robust solution.
Building software that falls over because someone exercised a perfectly
legitimate feature of another part of the system just isn't my idea of
the way to build stuff. We've had to put up with some cases of that
because we didn't want to change the protocol to fix it --- but now is
our opportunity to fix it.
regards, tom lane
How about sending an INFO or special taged message to the client when there is a GUC change, and have report_changes as a GUC variable that controls it? --------------------------------------------------------------------------- Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > And where does it stop? There are about a dozen GUC variables that will > > break an application as a whole if they don't have the value expected by > > the application. Do we need to install guards against all of these? > > The issue in my mind is not what will break an application, but what > will break a client-side library. The application knows, in some sense, > what settings it has selected -- either because it did explicit SETs or > because it's coded expecting certain values to be supplied via the GUC > default mechanisms. And the server knows what values have been set, > too. But the client-side library is out of the loop. We need to bring > it into the loop, at least for the values it needs to know about (and > yes, I agree that that's not a very well-defined set, but we can easily > set up the protocol to allow an expansible set of variables to be > transmitted). > > I think that "don't do that" is not an acceptably robust solution. > Building software that falls over because someone exercised a perfectly > legitimate feature of another part of the system just isn't my idea of > the way to build stuff. We've had to put up with some cases of that > because we didn't want to change the protocol to fix it --- but now is > our opportunity to fix it. > > regards, tom lane > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faqs/FAQ.html > -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes: > How about sending an INFO or special taged message to the client when > there is a GUC change, and have report_changes as a GUC variable that > controls it? just brainstorming. But if you're changing the on-wire representation, perhaps every transaction should just have a autocommit flag carried with it. Then you don't need any persistent setting, and the library doesn't need to be editing the text of transactions to insert BEGIN/COMMITs, the library just needs to either set the flag or not on every transaction. -- greg
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> How about sending an INFO or special taged message to the client when
> there is a GUC change, and have report_changes as a GUC variable that
> controls it?
Having such a variable would break the client libraries that need the
information. They won't stop needing the info just because some DBA
thinks it's a good idea to save a few bytes of bandwidth ...
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > How about sending an INFO or special taged message to the client when > > there is a GUC change, and have report_changes as a GUC variable that > > controls it? > > Having such a variable would break the client libraries that need the > information. They won't stop needing the info just because some DBA > thinks it's a good idea to save a few bytes of bandwidth ... You could configure it so once it is set by the client, only the client can change it, meaning it doesn't read from postgresql.conf. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>>> How about sending an INFO or special taged message to the client when
>>> there is a GUC change, and have report_changes as a GUC variable that
>>> controls it?
>>
>> Having such a variable would break the client libraries that need the
>> information. They won't stop needing the info just because some DBA
>> thinks it's a good idea to save a few bytes of bandwidth ...
> You could configure it so once it is set by the client, only the client
> can change it, meaning it doesn't read from postgresql.conf.
I'm not seeing the point, though. The amount of bandwidth involved is
insignificant, so there's no value in turning it off. AFAICT Peter's
objection to adding this is complexity, not bandwidth --- and adding a
control knob as you suggest will only make it even more complex.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>> How about sending an INFO or special taged message to the client when > >>> there is a GUC change, and have report_changes as a GUC variable that > >>> controls it? > >> > >> Having such a variable would break the client libraries that need the > >> information. They won't stop needing the info just because some DBA > >> thinks it's a good idea to save a few bytes of bandwidth ... > > > You could configure it so once it is set by the client, only the client > > can change it, meaning it doesn't read from postgresql.conf. > > I'm not seeing the point, though. The amount of bandwidth involved is > insignificant, so there's no value in turning it off. AFAICT Peter's > objection to adding this is complexity, not bandwidth --- and adding a > control knob as you suggest will only make it even more complex. My basic idea was using INFO-like message to send the SET change information. -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> My basic idea was using INFO-like message to send the SET change
> information.
Well, that's not really materially different from what I was proposing,
except that I was thinking of a format in which the variable name and
value are separate fields of a protocol message. If we send it out
as an INFO-message-like thingy then the client library has to parse
that string...
regards, tom lane
Tom Lane wrote: > I'm not convinced that it's worth the trouble to make the set of > reported variables be configurable on-the-fly as Bruce was suggesting. > client_encoding and datestyle are candidates to send this way, but are > there really others? Well, let's look at this from a long term perspective. How likely is it that a new development in the server will require additional monitoring on the client side? As an example, in the discussion about tracking transaction state, I haven't seen any mention of nested transactions, but I suspect that at least some clients (or their libraries) will be interested in the current transaction nesting level. What about savepoints? Is this something that the client side is going to have any interest in? I wouldn't want to dismiss that possibility entirely. Other features that we develop in the future may be things the client side is going to be interested in monitoring somehow, too. Here's where I'm going with this: because the needs of the client libraries are not set in stone for all time, and may even vary significantly depending on the client (the needs of pgadmin probably differ considerably from the needs of libpq, for instance), it seems to me that what we really need is a *general* mechanism for making it possible for the client to define what information it's interested in. In this case, what we're really talking about is a general monitoring function. So: the most sensible course of action seems, to me, to be to provide a mechanism to let the client notify the server that it should monitor the state of some variable, and the server should send a NOTICE (or some other kind of notification) to the client whenever the variable changes. If the SQL standard doesn't define a command for this, we'll have to define our own (something like "WATCH <variable> ON|OFF" or something...I'm sure someone can come up with a much nicer syntax than that but you get the point). > A point that would have to be considered is what to do when a SET > operation is rolled back by transaction abort. I think what we'd need > to do is retransmit each time the effective value changes; so it'd > work like this: > > -- initial DateStyle is foo > BEGIN; > SET DateStyle TO bar; > -- backend sends status message showing DateStyle = bar > SELECT ... > -- reported dates will use DateStyle bar > ROLLBACK; > -- backend sends status message showing DateStyle = foo Yes...whenever the watched variable changes from the perspective of the client, the change notice should be sent to the client. -- Kevin Brown kevin@sysexperts.com
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > My basic idea was using INFO-like message to send the SET change > > information. > > Well, that's not really materially different from what I was proposing, > except that I was thinking of a format in which the variable name and > value are separate fields of a protocol message. If we send it out > as an INFO-message-like thingy then the client library has to parse > that string... My idea of using a string is that it is easier to pass up to application that use libpq, like psql. Psql need to know about client encoding changes. -- 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