Re: Tepid: selective index updates for heap relations

On Fri, Jul 17, 2026, at 4:12 PM, Bharath Rupireddy wrote:
> Hi,
>
> On Thu, Jul 16, 2026 at 3:05 AM Greg Burd <greg(at)burd(dot)me> wrote:
>>
>> Rebased onto 3cf5264557b to address conflicts, no other changes.
>
> Thanks for working on this. I previously played around with PHOT and
> WARM a bit and built some context, but I need to refresh my memory and
> give things a re-read.

Thanks for looking at it!

> The patches cover a lot. Reducing the diff to the core and posting the
> stats, amcheck, and logical replication patches later would make
> review easier. I managed to get through 0001 and 0002 so far.

Yes, the 0001 and 0002 work is really a direct re-posting of the same two patches on my cf-5556 work that moves HeapDetermineColumnsInfo() into the executor. That was necessary work for the "HOT for Expression Indexes" but even more so for this work because this involves changing the contract between the table AM and the index AM as goverened by the executor to allow for cases other than TU_All/Summarizing/None.

Yes, it is dense and I should try to chop it up a bit more. I think at this stage it the viability of the idea hinges on the format changes and their long term implications. If those aren't accepted (and I'm on the fence myself on the "bit14" idea) then I have to get other foundational changes in before this could fly so I wasn't super concerned about having the final patch layout.

The tight integration of the TID abstraction across the table AM, the index AM and the executor is something that is hard to ignore and hard to work around. The TID abstraction is a heap specific thing (IMO) that works very well but is a very leaky and at times a highly limiting one.

> I would be more interested in focusing the initial discussion on the
> key design decisions. Where to store the modified index bitmap (it is
> per-row version). How we get vacuuming right. How bitmap scans and
> index scans produce correct results. How on-disk row format changes
> are acceptable. How upgrades work for migrating existing databases.
> How replication (logical and streaming) works and any impact on
> downstream systems (replicas, subscribers, etc.). How it impacts
> tooling (in-core like amcheck, pg_upgrade, pageinspect, and external
> ones).

Yes, all good topics.

> Most importantly, the trade-offs. We are going to have more heap bloat
> at the cost of less index bloat, which is fine since most OLTP
> workloads go via indexes, but does it impact analytical workloads that
> scan the heap? How does it impact vacuum performance? Do we know how
> many customers in practice modify indexed columns (not exact numbers,
> but some data points to keep the motivation up for this feature)? The
> fact that the modified index column bitmap is now stored per tuple
> version means fewer rows fit per page, which means more pages per
> relation, and does it also mean fewer HOT updates because updated rows
> cannot fit in the same page since some space is used for storing the
> bitmap, causing more index maintenance in turn?

Yes, there are trade-offs to call out. The heap pages will a) have more meta-data on them in the form of the bitmaps on UPDATE that collapse into dead tuples and b) yes there will be longer redirect chains that are harder to fully collect during vacuum and they will prevent pages from being marked frozen more frequently. That is true, and that has a cost.

> Also, the naming (SIU, Tepid, HOT-INDEXED, HEAP_INDEXED_UPDATED, and
> previously PHOT, WARM) is a bit confusing to me. Could we simplify the
> names and use them consistently?

I agree, and naming is hard. Happy to take on any ideas here. I've been calling it "tepid" and explaining it as "selective index updates" and in a way that is "partially HOT (PHOT)" but in other ways the tepid model isn't a "heap-only tuple (HOT)" at all, it's not heap-only it is heap and SOME indexes but not ALL. IDK what to call it, but maybe that's something to work out if/when the idea(s) and trade-offs are accepted and we're finalizing the code for commit?

> Some quick comments on the patches.
>
> 0001:
>
> 1/ Can 0001 be discussed in a separate thread? It seems to provide
> good coverage for HOT updates on its own and is worth discussing and
> perhaps getting committed separately.

This was pre-amble for changes that I'd planned for the HOT for Expressions patches. It's not strictly necessary in tepid, I should just drop it.

> 2/
>
> +SELECT id FROM hot_xml_test WHERE xpath('/person/name/text()', doc) =
> ARRAY['Alice2'::text];
> +ERROR: operator does not exist: xml[] = text[]
> +LINE 1: ..._xml_test WHERE xpath('/person/name/text()', doc) = ARRAY['A...
> + ^
> +DETAIL: No operator of that name accepts the given argument types.
> +HINT: You might need to add explicit type casts.
>
> +INSERT INTO hot_xml_test VALUES
> + (1, '<person><name>Alice</name><age>30</age></person>'),
> + (2, '<person><name>Bob</name><age>25</age></person>');
> +ERROR: could not identify a comparison function for type xml
> +SELECT * FROM get_hot_count('hot_xml_test');
>
> Are these expected?

Yeah no, my mistake. The more I looked at the XML tests the more they didn't really add value so I've removed them.

> 0002:
>
> 1/
> -SELECT * FROM base_tbl;
> +SELECT * FROM base_tbl ORDER BY a;
>
> ERROR: cannot insert a non-DEFAULT value into column "b"
> DETAIL: Column "b" is a generated column.
> -SELECT * FROM gtest1v;
> +SELECT * FROM gtest1v ORDER BY a;
>
> -DELETE FROM main_view WHERE a IN (20,21);
> +DELETE FROM main_view WHERE a = 20 AND b = 31;
> NOTICE: main_view BEFORE DELETE STATEMENT (before_view_del_stmt)
> NOTICE: main_view INSTEAD OF DELETE ROW (instead_of_del)
> -NOTICE: OLD: (21,10)
> -NOTICE: main_view INSTEAD OF DELETE ROW (instead_of_del)
> NOTICE: OLD: (20,31)
> +NOTICE: main_view AFTER DELETE STATEMENT (after_view_del_stmt)
> +DELETE 1
>
> The commit message says this fixes nondeterministic behavior in
> existing tests due to row ordering. I think these are unrelated to
> this work and could be discussed and committed separately.

Sure, possibly. The instability became apparent when working on the first two patches.

> 2/ ExecUpdateModifiedIdxAttrs() replaces HeapDetermineColumnsInfo().
> Why do we need to move modified index attribute computation to the
> executor and make every TTS and table AM pay that cost? HOT and
> modified index attributes are purely heap AM specific. If the executor
> ever needs the list of modified index attributes, why not let the AMs
> provide it as an out parameter (similar to how we pass the HOT hint in
> TU_UpdateIndexes format)?

The contract between the index and table AMs is governed by the the executor, or it should be (IMO), and the TU_All/Summarizing/None model that exists now is very much a heap-ism (leaky, too heap-MVCC specific) and so breaks down in the face of any table AM that has a different MVCC model. IMO the executor should be defaulting to only updating the indexes that are impacted by an update as a rule but allow for table AMs to influence that. Why? Because a table-agnostic executor should have as a goal not updating indexes unless those updates are required by the modifications underway. Doing more than that amount of work as a rule is assuming things the executor shouldn't know about in the table and/or index implementations. So, the executor (after patch 0002) will only update indexes that overlap with the modified attributes.

> I read the commit message saying that finding this set of attributes
> is not heap-specific but more general to all table AMs and could
> inform other decisions about when index inserts are required. But is
> it needed for this feature? If not, I think it can be discussed
> separately.

Yes, how else is the heap supposed to be able to communicate which subset of indexes to update in a generic manner? I tried other methods and they all felt (to me) like they were working around a leaky abstraction rather than fixing the root cause. Your view may differ.

> 3/
> - SELECT FROM injection_points_detach('heap_update-before-pin');
> - SELECT FROM injection_points_wakeup('heap_update-before-pin');
> + SELECT FROM injection_points_detach('simple_heap_update-before-pin');
> + SELECT FROM injection_points_wakeup('simple_heap_update-before-pin');
>
> Once we find the need for 0002, can we just leave the injection point
> name as-is to reduce the mechanical diff?

I can re-try and find out if it matters.

> 4/ Nits.
> Typos.
> + * are in the UPDATE statment and are known to be referenced by at least one
> + * ExecGetAllUpdatedCols(). Desipte the name it provides the set of

Fixed.

> No need to specify test names in the comments, because they can change anytime.

I disagree this time because the comment calls out a very very subtle hidden issue that is induced by that test and I felt should be documented so the next hacker could understand and avoid it earlier in the process than I did.

> * heap_modifiy_tuple(). There is one test in tsearch.sql that does just
> + * that, modifies an indexed attribute that isn't specified in the SQL and
>
> 5/
> + /* attidx is zero-based, attrnum is the normal attribute number */
> + AttrNumber attrnum = attidx + FirstLowInvalidHeapAttributeNumber;
>
> Is every TTS implementer expected to support all system columns that
> FirstLowInvalidHeapAttributeNumber implies? Asking because 0002 moved
> this code to the executor in ExecCompareSlotAttrs.

Interesting point. Yes, I did move a heap-ism into the executor (facepalm), something I claim to be against. :)

I'm going to take the abstraction critique seriously and move the mechanism of "which attributes changed" behind the slot/table-AM boundary, while keeping the policy "only maintain indexes whose attributes overlap the change" in the executor.

Concretely: ExecCompareSlotAttrs() as written leaks heap assumptions upward (it enumerates over FirstLowInvalidHeapAttributeNumber and hand-handles system columns like tableoid), which is fair to call out. The comparison of two versions of a row is something the AM should answer for its own attributes and its own notion of "a version changed"; the executor should only collect that changed-set and intersect it with each index's attribute set to decide which indexes need fresh entries. That split is the honest form of "the executor governs the table-AM/index-AM contract, the table AM owns its storage mechanics."

The reason I can't instead adopt the suggested out-parameter model, have the AM return modified_attrs from table_tuple_update(), is a matter of ordering, and I think it's the same ordering that a couple of the concurrency questions in the thread are also tripping on, so it's worth stating plainly:

modified_attrs is an input to table_tuple_update(), not a byproduct of it. The heap AM reads it during the update to decide, before it writes anything, (a) whether the update can stay HOT / selectively-indexed vs. must update all indexes (HeapUpdateHotAllowable), (b) the tuple lock mode (HeapUpdateDetermineLockmode), and (c) whether the replica-identity key changed. The executor then uses the same set after the update to drive which indexes get fresh entries. A value returned from the update call would arrive after every one of those decisions had already been made it's structurally too late to inform them. So the changed-attribute set has to be available to, and passed into, the table AM, not produced by it.

The clean way to satisfy both constraints that comes to mind is a slot/AM-level comparison the executor calls to build the set (AM owns "what changed"), then passes into the update (executor owns "which indexes"). That keeps the decision points in the right order and removes the heap-ism from the executor. To that end I've added to the table AM: table_modified_attrs() callback in an updated 0002 and in heap heapam_modified_attrs() in a new 0003 patch.

(Separately and related to that: this is also why the "reduces buffer-lock hold time" line in the 0002 commit message is both wrong and misleading the computation was always pre-lock and I'm dropping it.)

> 6/ The commit message says that having ExecUpdateModifiedIdxAttrs() in
> the executor reduces the time the buffer lock is held by computing
> modified index columns before table_tuple_update(). How is this
> correct from a concurrency perspective? If another transaction
> modifies the same tuple between when the executor compares columns and
> when heap_update acquires the buffer lock, what happens? The executor
> locks the old tuple explicitly only when it detects concurrent updates
> or deletes to the same tuple, but does not hold the tuple lock the
> first time.

Good catch, and the commit message is misleading here I'll reword it. The "reduces the time the buffer lock is held" line oversells a benefit that isn't real and, worse, invites exactly the concurrency worry you raised. In today's tree HeapDetermineColumnsInfo() already runs before heap_update() takes the buffer lock (it's computed up front from oldtup/newtup), so moving the computation into the executor doesn't change when, relative to the buffer lock, the comparison happens. The motivation for the move is the table-AM/index-AM contract, not lock-hold latency; I'll drop that paragraph so it stops implying otherwise.

On the concurrency correctness itself, there's no window to exploit:

- ExecUpdateAct() computes modified_attrs from (oldSlot, newSlot) and immediately calls table_tuple_update() in the same function, with no lock dropped and no yield between the two. There is no "compare, then later acquire the buffer lock" gap — the comparison is not protected by, nor waiting on, any buffer lock, in either the old or new arrangement.

- Concurrency is still detected exactly where it always was: inside heap_update(), under the buffer lock, via the xmax/visibility check. If another transaction updated the row first, heap_update() returns TM_Updated (unchanged by this patch).

- On TM_Updated, ExecUpdate() runs the normal EPQ path: table_tuple_lock() the latest version, EvalPlanQual(), re-fetch the latest oldSlot and rebuild the new slot, then goto redo_act — which re-enters ExecUpdateAct() and recomputes modified_attrs against the fresh versions.

- modified_attrs is consumed only in ExecUpdateEpilogue() (the index-insert step), which runs only on the TM_Ok path. A modified_attrs computed against a version that lost the race is discarded and recomputed before any index maintenance happens.

So the set of indexes we maintain is always derived from the same (old, new) pair the update was actually applied to; a concurrent update forces a re-read and recompute before index inserts, identical in effect to how HeapDetermineColumnsInfo() behaves today. The executor doesn't need to hold a tuple lock across the comparison for the same reason it doesn't today — the AM's TM_Updated/EPQ protocol is the serialization point.

I'll fix the injection-point/comment nits and drop the misleading commit-message paragraph in the next revision.

> I will continue reading the other patches in the coming weeks.
>
> --
> Bharath Rupireddy
> Amazon Web Services: https://aws.amazon.com

best.

-greg