public inbox for [email protected]
help / color / mirror / Atom feedFrom: Noah Misch <[email protected]>
To: Nitin Motiani <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Cc: Smolkin Grigory <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Subject: Re: race condition in pg_class
Date: Thu, 5 Sep 2024 15:04:34 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH5HC96ZwrjKF-RBijntw9uX2UzUtEKC3A7cxB_Xh8arVuKhKw@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CAH5HC94OEarC9=FbS0o4D5DYKODMeYjVJ_KxxcBRQFMiMAOfZg@mail.gmail.com>
<[email protected]>
<CAH5HC96yPfrLVQ-srZ65R73t_dBLNM4mCR-RGcT5zNa04DHSxA@mail.gmail.com>
<[email protected]>
<CAH5HC96ASNkXyYVEG7pO98qr1WARGTohxGjVpBsu4c3hkM_OcA@mail.gmail.com>
<[email protected]>
<CAH5HC96ZwrjKF-RBijntw9uX2UzUtEKC3A7cxB_Xh8arVuKhKw@mail.gmail.com>
On Thu, Sep 05, 2024 at 07:10:04PM +0530, Nitin Motiani wrote:
> On Thu, Sep 5, 2024 at 1:27 AM Noah Misch <[email protected]> wrote:
> > On Wed, Sep 04, 2024 at 09:00:32PM +0530, Nitin Motiani wrote:
> > > Assert(rel->ri_needsLockTagTuple == IsInplaceUpdateRelation(rel->relationDesc)
> > >
> > > This can safeguard against users of ResultRelInfo missing this field.
> >
> > v10 does the rename and adds that assertion. This question remains open:
>
> Looks good. A couple of minor comments :
> 1. In the inplace110 commit message, there are still references to
> heap_inplace_update. Should it be clarified that the function has been
> renamed?
PGXN has only one caller of this function, so I think that wouldn't help
readers enough. If someone gets a compiler error about the old name, they'll
figure it out without commit log guidance. If a person doesn't get a compiler
error, they didn't need to read about the fact of the rename.
> 2. Should there be a comment above the ri_needLockTag definition in
> execNodes.h that we are caching this value to avoid function calls to
> IsInPlaceUpdateRelation for every tuple? Similar to how the comment
> above ri_TrigFunctions mentions that it is cached lookup info.
Current comment:
/* updates do LockTuple() before oldtup read; see README.tuplock */
bool ri_needLockTagTuple;
Once the comment doesn't fit in one line, pgindent rules make it take a
minimum of four lines. I don't think words about avoiding function calls
would add enough value to justify the vertical space, because a person
starting to remove it would see where it's called. That's not to say the
addition would be negligent. If someone else were writing the patch and had
included that, I wouldn't be deleting the material.
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: race condition in pg_class
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox