public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Greg Burd <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Jeff Davis <[email protected]>
Subject: Re: Expanding HOT updates for expression and partial indexes
Date: Thu, 12 Mar 2026 15:33:15 -0500
Message-ID: <abMjC0jifWB0cs5F@nathan> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<akciabcu3b2hchj7adxhu4kovfaozp2pcn2z7sdljfthxcyg4o@7e6sfyzipvyy>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On Wed, Mar 11, 2026 at 11:51:03AM -0400, Greg Burd wrote:
> 0002 - This patch plugs a hole (bug?) in ExecGetAllUpdatedCols() which is
> triggered by an existing test in tsearch.sql and the
> tsvector_update_trigger(). That trigger uses heap_modify_tuple() to
> change an indexed attribute that is not discovered by
> ExecGetAllUpdatedCols(), which seems odd to me at best and at worst wrong
> (or even a potential security issue). This patch finds and adds columns
> that are updated into the Bitmapset returned by ExecGetAllUpdatedCols().
> The patch includes a helper function ExecCompareSlotAttrs() that will be
> used in follow-on patches as well.
I just looked at this one for now.
> The net is that the functions like HeapDetermineColumnsInfo() have to
> scan all indexed attributes for changes rather than being able to first
> reduce the indexed set by intersecting it with the set of attributes
> known to be potentially updated.
I noticed the patch doesn't update HeapDetermineColumnsInfo() accordingly.
Is that intended?
> This commit introduces ExecCompareSlotAttrs() as a utility function to
> identify those attributes that have changed. It compares a subset of
> attributes between two TupleTableSlots and returns a Bitmapset of
> attributes that differ.
Hm. Most of this new function looks duplicated from
HeapDetermineColumnsInfo(), so IIUC this commit effectively adds another
scan through all the attributes. Does this produce noticeably more
overhead?
> It would be nice to integrate this into HeapDetermineColumnsInfo(),
> however it would be a layering violation given that it is within
> heap_update().
It'd be good to understand whether the current behavior is intentional or
just a happy accident. I found commit 2fd8685e7f, which looks like it was
intended as a prerequisite for the WARM feature (which I don't think was
ever committed). And it seems to have scanned through all indexed columns
when HOT was first introduced in commit 282d2a03dd.
I'm also curious whether anything else could modify columns that won't be
discovered by ExecGetAllUpdatedCols(). Having HeapDetermineColumnsInfo()
scan everything seems like a defense against such things, which is perhaps
why you've left it unchanged in the patch. I haven't looked into 0003 yet.
Is 0002 a prerequisite for that or a separate fix?
--
nathan
view thread (44+ 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]
Subject: Re: Expanding HOT updates for expression and partial indexes
In-Reply-To: <abMjC0jifWB0cs5F@nathan>
* 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