Message-ID: <96b451774d927daffe7fe40d06447eabb06a8f3f.camel@j-davis.com>
Subject: Re: Expanding HOT updates for expression and partial indexes
From: Jeff Davis <pgsql@j-davis.com>
To: Nathan Bossart <nathandbossart@gmail.com>, Greg Burd <greg@burd.me>
Cc: "Burd, Greg" <gregburd@amazon.com>, "pgsql-hackers@postgresql.org"
	 <pgsql-hackers@postgresql.org>
Date: Thu, 09 Oct 2025 12:08:30 -0700
In-Reply-To: <aObOLg3JsebbwjOU@nathan>
References: <97f0aa72-f172-4673-8b04-533f022c3149@app.fastmail.com>
	 <CF007F8C-5FCA-4FC5-A7EC-282F8C9D413C@greg.burd.me>
	 <aObOLg3JsebbwjOU@nathan>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.3-0ubuntu1 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/96b451774d927daffe7fe40d06447eabb06a8f3f.camel%40j-davis.com>
Precedence: bulk

On Wed, 2025-10-08 at 15:48 -0500, Nathan Bossart wrote:
> > The theory being that
> > my new code using the old/new tts to form and test the index tuples
> > resulting from executing expressions was using the resultsRelInfo
> > struct
> > created during plan execution, not the information found on the
> > page,
> > and so was safe without the lock.
>=20
> An open question (at least from me) is whether this is safe.=C2=A0 I'm no=
t
> familiar enough with this area of code yet to confidently determine
> that.

The optimization requires that the expression evaluates to the same
thing on the old and new tuples. That determination doesn't have
anything to do with a lock on the buffer, so long as the old tuple
isn't pruned away or something. And clearly it won't be pruned, because
we're in the process of updating it, so we have a snapshot that can see
it.

There might be subtleties in other parts of the proposal, but the above
determination can be made safely without a buffer lock.

>=20
> > I added a reloption "expression_checks" to disable this new code
> > path.=20
> > Good idea or bad precedent?
>=20
> If there are cases where the added overhead outweighs the benefits
> (which
> seems like it must be true some of the time), then I think we must
> have a
> way to opt-out (or maybe even opt-in).=C2=A0 In fact, I'd advise adding a
> GUC to
> complement the reloption so that users can configure it at higher
> levels.

I'll push back against this. For now I'm fine with developer options to
make testing easier, but we should find a way to make this work well
without tuning.

Regards,
	Jeff Davis