public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Greg Burd <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Expanding HOT updates for expression and partial indexes
Date: Tue, 17 Mar 2026 10:22:02 -0500
Message-ID: <ablxmmcbA_8UFjiN@nathan> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<abMjC0jifWB0cs5F@nathan>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
Catching up here. I see that you dropped 0002. Can you explain why that's
no longer needed?
On Mon, Mar 16, 2026 at 04:51:31PM -0400, Greg Burd wrote:
> Refactor executor update logic to determine which indexed columns have
> actually changed during an UPDATE operation rather than leaving this up
> to HeapDetermineColumnsInfo() in heap_update(). Finding this set of
> attributes is not heap-specific, but more general to all table AMs and
> having this information in the executor could inform other decisions
> about when index inserts are required and when they are not regardless
> of the table AM's MVCC implementation strategy.
Nice, this is a crisp motivation statement.
> Development of this feature exposed nondeterministic behavior in three
> existing tests which have been adjusted to avoid inconsistent test
> results due to tuple ordering during heap page scans.
Logistically speaking, these could be nice to get out of the way early as a
prerequisite patch so we can focus on the substance of this patch.
The rest of my comments are from a relatively quick skim. Deeper review to
follow...
> + /*
> + * Reduce the set under review to only the unmodified indexed replica
> + * identity key attributes. idx_attrs is copied (by bms_difference())
> + * not modified here.
> + */
> + attrs = bms_difference(idx_attrs, modified_idx_attrs);
> + attrs = bms_int_members(attrs, rid_attrs);
> +
> + while ((attidx = bms_next_member(attrs, attidx)) >= 0)
Could it be worth moving this loop (and some surrounding code) to a helper
function?
> - * Note: beyond this point, use oldtup not otid to refer to old tuple.
> + * NOTE: beyond this point, use oldtup not otid to refer to old tuple.
nitpick: Please remove unnecessary changes.
> @@ -5269,10 +5269,10 @@ RelationGetIndexPredicate(Relation relation)
> * in expressions (i.e., usable for FKs)
> * INDEX_ATTR_BITMAP_PRIMARY_KEY Columns in the table's primary key
> * (beware: even if PK is deferrable!)
> + * INDEX_ATTR_BITMAP_SUMMARIZED Columns only included in summarizing indexes
> * INDEX_ATTR_BITMAP_IDENTITY_KEY Columns in the table's replica identity
> * index (empty if FULL)
> - * INDEX_ATTR_BITMAP_HOT_BLOCKING Columns that block updates from being HOT
> - * INDEX_ATTR_BITMAP_SUMMARIZED Columns included in summarizing indexes
> + * INDEX_ATTR_BITMAP_INDEXED Columns referenced by indexes
Is the meaning of INDEX_ATTR_BITMAP_SUMMARIZED changing in this patch? I
see you moved it and dropped the "only".
> - Bitmapset *summarizedattrs; /* columns with summarizing indexes */
> + Bitmapset *indexedattrs; /* columns referenced by indexes */
> + Bitmapset *summarizedattrs; /* columns only in summarizing indexes */
But you added an "only" here...
--
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: <ablxmmcbA_8UFjiN@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