Re: Surprising behaviour of \set AUTOCOMMIT ON

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Sep 15, 2016 at 11:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Thu, Sep 15, 2016 at 7:29 AM, Rahila Syed <rahilasyed90(at)gmail(dot)com> 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 current transaction has ended.

>>> Hmm, that seems like a reasonable compromise.

>> I dunno, implementing that seems like it will require some very fragile
>> behavior in the autocommit code, ie that even though the variable is "on"
>> we don't do anything until after reaching an out-of-transaction state.
>> It might work today but I'm afraid we'd break it in future.

> Hmm, I don't think any logic change is being proposed, just a warning
> that it may not work the way you think. So I don't think it would be
> any more fragile than now. Am I missing something?

As I see it, up to now we never considered what would happen if you
changed the variable's setting mid-transaction. The fact that it works
like this is an implementation artifact. Now that we are considering it,
we should ask ourselves if we like that artifact and are willing to commit
to keeping it working like that forevermore. I'm not sure that the answer
to either one is "yes". I think throwing an actual error would be
preferable.

>> 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"?

> I agree that'd be better but I don't know that we should expect Rahila
> to do that as a condition of getting a usability warning accepted.

If it's something that would end up getting thrown away after someone
does the API change, accepting a warning-only patch doesn't seem all
that useful. But I just work here.

regards, tom lane