On Tue, Nov 19, 2013 at 1:14 PM, Bruce Momjian <bruce@momjian.us> wrote:
> On Tue, Nov 19, 2013 at 07:12:32PM +0100, Andres Freund wrote:
>> On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote:
>> > > Why change the historical behaviour for savepoints?
>> >
>> > Because as Tom stated, we already do warnings for other useless
>> > transaction commands like BEGIN WORK inside a transaction block:
>>
>> Which imo is a bad, bad historical accident. I've repeatedly seen this
>> hide bugs causing corrupted data in the end.
>>
>> But even if that weren't a concern, the fact that BEGIN does it one way
>> currently doesn't seem very indicative of changing other historical behaviour.
>
> Look at this gem, which returns notice:
>
> test=> ABORT;
> NOTICE: there is no transaction in progress
> ROLLBACK
> test=>
>
> We are all over the map on this! The big question is whether we want to
> add some sanity to this, or just leave it alone, and if we leave it
> alone, what pattern do we use for the new checks?
I think the pattern is and should be different for toplevel
transaction control commands than for other things. If you issue a
BEGIN, we want it to end up that you're definitely in a transaction at
that point, and if you issue a COMMIT or ROLLBACK or ABORT, we want
you to definitely be out of a transaction after that. This is
important for reasons discussed on Andrew's thread about pre-commit
triggers just today.
The same considerations don't apply elsewhere; the user has made a
mistake, and there's no particular reason not to throw an ERROR. We
could throw a WARNING or NOTICE and pretend like things are OK, but
there doesn't seem to be much point, certainly not enough to justify
changing long-established behavior.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
