Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> Well, if you think there's a real backwards compatibility issue, we
>> should just do #2 and be done with it. It's not like it's enough code
>> to really matter in the big scheme of things.
> I don't like it just because it's another kludge in the way we set up
> ActiveSnapshot. I think it would be better if we were simplifying that
> code, not adding more kludges.
> If there's no backwards compatibility argument (and from the looks of
> your patch, perhaps there wouldn't), then I think we should just do #1.
On the whole I think your original instinct was right: there is a
backwards compatibility issue here. Without the kluge added to
trigger.c, this would fail:
BEGIN;SET CONSTRAINTS ALL IMMEDIATE;SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
since a transaction snapshot would be set before reaching the isolation
level change. Since that has worked in the past, it seems there's a
nonnegligible risk of breaking apps. There's no obvious-to-the-user
reason why this ordering shouldn't be okay ...
regards, tom lane