Re: BUG #17385: "RESET transaction_isolation" inside serializable transaction causes Assert at the transaction end

On Thu, Mar 10, 2022 at 8:36 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Thu, Mar 10, 2022 at 12:11 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> writes:
> > > On Wed, Feb 16, 2022 at 1:55 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >> Yeah, I was considering that too. A new GUC_NO_RESET flag would be
> > >> cheaper than running the check hooks during RESET, and probably
> > >> safer too. On the other hand, we would lose the property that
> > >> you can reset these settings as long as you've not yet taken a
> > >> snapshot. I wonder whether there is any code out there that
> > >> depends on that ...
> >
> > > Indeed. I guess that it's relatively common that the transaction
> > > isolation level is set after BEGIN TRANSACTION but I've not heard that
> > > it's reset after BEGIN TRANSACTION with setting non-default
> > > transaction isolation level.
> >
> > Yes, we certainly have to preserve the SET case, but using RESET
> > in that way seems like it'd be pretty weird coding. I'd have no
> > hesitation about banning it in a HEAD-only change. I'm slightly
> > more nervous about doing so in a back-patched bug fix. On the
> > other hand, starting to call check hooks in a context that did
> > not use them before is also scary to back-patch.
>
> Agreed.
>
> For back branches, it might be less scary to call check hooks for only
> limited GUCs such as transaction_isolation.
>
> > Do you want to draft up a patch that fixes this with a new
> > GUC flag?
>
> Yes, I'll update the patch accordingly.

I've attached a draft patch for discussion.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/