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

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dmitry Koval <d.koval@postgrespro.ru>, Dilip Kumar <dilipbalaut@gmail.com>, andrewbille@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Date: 2022-03-11T15:39:53Z
Lists: pgsql-bugs

Attachments

On Thu, Mar 10, 2022 at 8:36 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Mar 10, 2022 at 12:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Masahiko Sawada <sawada.mshk@gmail.com> writes:
> > > On Wed, Feb 16, 2022 at 1:55 AM Tom Lane <tgl@sss.pgh.pa.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/

Commits

  1. Renumber GUC flags for a bit more sanity.

  2. Introduce GUC_NO_RESET flag.