Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Tom Lane
"Daniel Verite" writes: > It looks easy to make the hooks return a bool, and when returning false, > cancel the assignment of the variable. Yeah, that's about how I was envisioning it too. > I volunteer to write that patch. Thanks. > It would close the hazard that

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Daniel Verite
Tom Lane wrote: > I think changing the hook API is a pretty reasonable thing to do here > (though I'd make it its own patch and then add the autocommit change > on top). When was the last time you actually wanted to set VERBOSITY > to "fubar"? It looks easy to make the hooks return a

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Tom Lane
Robert Haas writes: > On Thu, Sep 15, 2016 at 11:10 AM, Tom Lane wrote: >> Robert Haas writes: >>> On Thu, Sep 15, 2016 at 7:29 AM, Rahila Syed wrote: In keeping with current design of hooks instead

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Robert Haas
On Thu, Sep 15, 2016 at 11:10 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Sep 15, 2016 at 7:29 AM, Rahila Syed wrote: >>> In keeping with current design of hooks instead of rejecting autocommit 'ON' >>> setting inside

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Tom Lane
Robert Haas writes: > On Thu, Sep 15, 2016 at 7:29 AM, Rahila Syed wrote: >> In keeping with current design of hooks instead of rejecting autocommit 'ON' >> setting inside >> a transaction,the value can be set to 'ON' with a psql_error displaying

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Robert Haas
On Thu, Sep 15, 2016 at 7:29 AM, Rahila Syed wrote: > In keeping with current design of hooks instead of rejecting autocommit 'ON' > setting inside > a transaction,the value can be set to 'ON' with a psql_error displaying that > the value > will be effective when the

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-15 Thread Rahila Syed
>Have you considered expanding >the API for hook functions? Changing the hooks API to allow rejecting a setting and return false is certainly useful to other psql variables wanting to report an error and reject a value. I did not consider expanding hook APIs because there was no requirement in

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-14 Thread Tom Lane
Rahila Syed writes: >> Looking at the other variables hooks, they already emit errors and can >> deny the effect of a change corresponding to a new value, without >> informing the caller. Why would autocommit be different? > These instances where psql_error occurs inside

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-14 Thread Rahila Syed
Hello, >Looking at the other variables hooks, they already emit errors and can >deny the effect of a change corresponding to a new value, without >informing the caller. Why would autocommit be different? The only type of psql_error inside hooks is as follows, psql_error("unrecognized value

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-08 Thread Daniel Verite
Rahila Syed wrote: > However, including the check here will require returning the status > of command from this hook. i.e if we throw error inside this > hook we will need to return false indicating the value has not changed. Looking at the other variables hooks, they already emit errors

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-08 Thread Rahila Syed
Thank you for comments. >But there's a function in startup.c which might be the ideal location >or the check, as it looks like the one and only place where the >autocommit flag actually changes: >static void >autocommit_hook(const char *newval) Thank you for pointing out this. This indeed is

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-03 Thread Tom Lane
"Daniel Verite" writes: > Rushabh Lathia wrote: >> so its good to have all the set variable related checks inside >> SetVariable(). > There's already another way to skip the \set check: > select 'on' as "AUTOCOMMIT" \gset Hmm, that might be a bug.

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-03 Thread Daniel Verite
Rushabh Lathia wrote: > It might happen that SetVariable() can be called from other places in > future, > so its good to have all the set variable related checks inside > SetVariable(). There's already another way to skip the \set check: select 'on' as "AUTOCOMMIT" \gset But there's a

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-03 Thread Daniel Verite
Rahila Syed wrote: > >Above error coming because in below code block change, you don't have > check for > >command (you should check opt0 for AUTOCOMMIT command) > Corrected in the attached. There are other values than ON: true/yes and theirs abbreviations, the value 1, and anything

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-02 Thread Rahila Syed
>Document doesn't say this changes are only for implicit BEGIN.. Correcting myself here. Not just implicit BEGIN, it will throw an error on \set AUTOCOMMIT inside any any transaction provided earlier value of AUTOCOMMIT was OFF. Probably the test in which you tried it was already ON. Thank you,

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-02 Thread Rushabh Lathia
On Fri, Sep 2, 2016 at 1:12 PM, Rahila Syed wrote: > Hello, > > Thank you for comments. > > >Above test not throwing psql error, as you used strcmp(newval,"ON"). You > >should use pg_strcasecmp. > Corrected in the attached. > > >Above error coming because in below code

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-02 Thread Amit Langote
Hi, Some minor comments. + + + Autocommit cannot be set on inside a transaction, the ongoing + transaction has to be ended by entering COMMIT or + ROLLBACK before setting autocommit on. + + I guess: "cannot be set *to* on" and likewise

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-02 Thread Rahila Syed
Hello, Thank you for comments. >Above test not throwing psql error, as you used strcmp(newval,"ON"). You >should use pg_strcasecmp. Corrected in the attached. >Above error coming because in below code block change, you don't have check for >command (you should check opt0 for AUTOCOMMIT command)

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-02 Thread Rushabh Lathia
I don't think patch is doing correct things: 1) postgres=# \set AUTOCOMMIT off postgres=# create table foo (a int ); CREATE TABLE postgres=# \set AUTOCOMMIT on Above test not throwing psql error, as you used strcmp(newval,"ON"). You should use pg_strcasecmp. 2) postgres=# \set AUTOCOMMIT OFF

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-09-01 Thread Rahila Syed
Ok. Please find attached a patch which introduces psql error when autocommit is turned on inside a transaction. It also adds relevant documentation in psql-ref.sgml. Following is the output. bash-4.2$ psql -d postgres psql (10devel) Type "help" for help. postgres=# \set AUTOCOMMIT OFF postgres=#

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-17 Thread Robert Haas
On Tue, Aug 16, 2016 at 5:25 PM, Rahila Syed wrote: >>I think I like the option of having psql issue an error. On the >>server side, the transaction would still be open, but the user would >>receive a psql error message and the autocommit setting would not be >>changed.

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-16 Thread Rahila Syed
>I think I like the option of having psql issue an error. On the >server side, the transaction would still be open, but the user would >receive a psql error message and the autocommit setting would not be >changed. So the user could type COMMIT or ROLLBACK manually and then >retry changing the

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-13 Thread Amit Kapila
On Sat, Aug 13, 2016 at 1:05 AM, David G. Johnston wrote: > > I'll admit I haven't tried to find fault with the idea (or discover better > alternatives) nor how it would look in implementation. As a user, though, > it would make sense if the system behaved in this

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-12 Thread David G. Johnston
On Fri, Aug 12, 2016 at 3:03 PM, Robert Haas wrote: > On Thu, Aug 11, 2016 at 8:34 AM, David G. Johnston > wrote: > > I don't have a fundamental issue with saying "when turning auto-commit on > > you are also requesting that the open

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-12 Thread Robert Haas
On Thu, Aug 11, 2016 at 8:34 AM, David G. Johnston wrote: > I don't have a fundamental issue with saying "when turning auto-commit on > you are also requesting that the open transaction, if there is one, is > committed immediately." I'm more inclined to think an error

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-11 Thread David G. Johnston
On Thu, Aug 11, 2016 at 2:11 AM, Venkata Balaji N wrote: > > ​[...] > committing all the previously open transactions > ​[...] > "All"? ​There can only ever be at most one open transaction at any given time... I don't have a fundamental issue with saying "when turning

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-11 Thread Venkata Balaji N
On Thu, Aug 11, 2016 at 2:58 PM, Venkata Balaji N wrote: > > On Tue, Aug 9, 2016 at 1:02 AM, Robert Haas wrote: > >> On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed >> wrote: >> > Thank you for inputs everyone. >> > >> > The

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-10 Thread Venkata Balaji N
On Tue, Aug 9, 2016 at 1:02 AM, Robert Haas wrote: > On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed > wrote: > > Thank you for inputs everyone. > > > > The opinions on this thread can be classified into following > > 1. Commit > > 2. Rollback > > 3.

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread Sridhar N Bamandlapally
Just for information, PG current behavior, "\set AUTOCOMMIT OFF" implicitly does/open "BEGIN;" block So, "\set AUTOCOMMIT ON" has no effect once "\set AUTOCOMMIT OFF" is issued until "END;" or "COMMIT;" or "ROLLBACK;" however, I think if exit session release the transactions then change

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread Vik Fearing
On 08/08/16 17:02, Robert Haas wrote: > On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed wrote: >> Thank you for inputs everyone. >> >> The opinions on this thread can be classified into following >> 1. Commit >> 2. Rollback >> 3. Error >> 4. Warning >> >> As per opinion

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread Robert Haas
On Mon, Aug 8, 2016 at 11:17 AM, David G. Johnston wrote: > On Mon, Aug 8, 2016 at 11:02 AM, Robert Haas wrote: >> >> On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed >> wrote: >> > Thank you for inputs everyone. >> > >> >

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread David G. Johnston
On Mon, Aug 8, 2016 at 11:02 AM, Robert Haas wrote: > On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed > wrote: > > Thank you for inputs everyone. > > > > The opinions on this thread can be classified into following > > 1. Commit > > 2. Rollback > > 3.

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread Robert Haas
On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed wrote: > Thank you for inputs everyone. > > The opinions on this thread can be classified into following > 1. Commit > 2. Rollback > 3. Error > 4. Warning > > As per opinion upthread, issuing implicit commit immediately after

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-08 Thread Rahila Syed
Thank you for inputs everyone. The opinions on this thread can be classified into following 1. Commit 2. Rollback 3. Error 4. Warning As per opinion upthread, issuing implicit commit immediately after switching autocommit to ON, can be unsafe if it was not desired. While I agree that its

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-06 Thread Matt Kelly
Its worth noting that the JDBC's behavior when you switch back to autocommit is to immediately commit the open transaction. Personally, I think committing immediately or erroring are unsurprising behaviors. The current behavior is surprising and obviously wrong. Rolling back without an error

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-06 Thread Brendan Jurd
As an extra data point, if you try this in Python (psycopg2) you get an exception: psycopg2.ProgrammingError: autocommit cannot be used inside a transaction I think this exception is a legitimate response. If the user switches on autocommit mode inside a transaction, it was most likely not on

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-06 Thread Pavel Stehule
2016-08-06 7:29 GMT+02:00 Amit Kapila : > On Thu, Aug 4, 2016 at 7:46 PM, Pavel Stehule > wrote: > > > 2016-08-04 15:37 GMT+02:00 Amit Kapila : > >> > >> > I dislike automatic commit or rollback here. > >> > > >> > >>

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-05 Thread Amit Kapila
On Thu, Aug 4, 2016 at 7:46 PM, Pavel Stehule wrote: > 2016-08-04 15:37 GMT+02:00 Amit Kapila : >> >> > I dislike automatic commit or rollback here. >> > >> >> What problem you see with it, if we do so and may be mention the same >> in docs as

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-04 Thread Pavel Stehule
2016-08-04 15:37 GMT+02:00 Amit Kapila : > On Wed, Aug 3, 2016 at 5:09 PM, Pavel Stehule > wrote: > > > > > > 2016-08-03 12:16 GMT+02:00 Rahila Syed : > >> > >> Should changing the value from OFF to ON automatically either

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-04 Thread Amit Kapila
On Wed, Aug 3, 2016 at 5:09 PM, Pavel Stehule wrote: > > > 2016-08-03 12:16 GMT+02:00 Rahila Syed : >> >> Should changing the value from OFF to ON automatically either commit or >> rollback transaction in progress? >> >> >> FWIW, running set

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

2016-08-03 Thread Pavel Stehule
2016-08-03 12:16 GMT+02:00 Rahila Syed : > Hello, > > Need community's opinion on following behaviour of \set AUTOCOMMIT > > If \set AUTOCOMMIT ON is issued after \set AUTOCOMMIT OFF the commands > which follow after AUTOCOMMIT is set ON are not committed until an