Re: Tepid: selective index updates for heap relations

From: Greg Burd <greg(at)burd(dot)me>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Subject: Re: Tepid: selective index updates for heap relations
Date: 2026-09-28 01:27:33
Message-ID: 3604ec98-147d-4ba5-b215-412fa2797633@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Mon, Jul 27, 2026, at 11:36 PM, Chao Li wrote:
>> On Jul 21, 2026, at 05:34, Greg Burd <greg(at)burd(dot)me> wrote:
>>
>>
>> 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<v67-tepid.tgz>
>
> Thanks for the patch.
>
> I have spent some time reviewing v67 0001-0009. git am failed at 0010,
> so a rebase is needed.

Hello Chao, thank you for investing time to review this work.

Apologies for the delay responding. It has been a long time since I've updated this patch set due to /reasons/ that don't matter but here. I'm back at it and hope you appreciate these changes in approach and in code.

> I think I have only got a rough idea of what this feature does and how
> it works so far. I believe I will still need much more time to
> understand the details.
>
> I just want to raise a few design comments I have so far:
>
> * Does it make sense to add a table-level option to make this feature
> opt-in? From my understanding, this feature will benefit tables that
> have many indexes across multiple fields. Although benchmarks are
> attached, I am afraid they may not be able to simulate all users' data
> models. Making it opt-in would allow users to easily evaluate the
> feature with their own tables and enable it only on tables that really
> benefit from it. I realize that turning it on is easy, but turning it
> off may require table rewritten and index rebuilds. However, I think
> that is acceptable, as many other ALTER TABLE operations may also
> require so.

I appreciate your point of view on this, it is a behavior change for heap and that means basically everyone. This feature benefits UPDATEs that modify some, but not all, of a relation's indexed columns: today those force a new entry in every index; with this change only the indexes whose columns changed are maintained. In those cases, where some but not all indexes on a relation intersect, the benefit is avoiding the index write and the overhead that "index bloat" creates for vacuum and index scans. Avoiding those writes avoids index "bloat" and the I/O and storage costs during the operation. This is measurable in terms of TPS for UPDATEs that fall into that pattern and also in terms of WAL volume, index size, and vacuum time and frequency.

I'm personally against a GUC or reloption or other gating because the feature is safe and beneficial by default, and a knob adds permanent surface area, we didn't gate the initial HOT feature so why do so now? That said, it's a good idea to explore the idea and answer your questions about it. I don't believe there would be any index corruption or issues if we had a knob to enable/disable the feature even after operating in one mode or the other for a while as long as the chain walking logic remained on, that is to say that the read side (chain walk + crossed-attribute recheck) is stateless w.r.t. the knob; only creation of new HOT-indexed updates would be gated. If that was also gated by the knob then existing chains age out through normal prune/vacuum; a VACUUM FULL or CLUSTER normalizes immediately but wouldn't be required.

> * When an index entry points to a stale tuple chain, it still returns
> the entry and sets xs_hot_indexed_stale, which means callers of
> index_getnext_slot() need to check this new state. This seems to weaken
> the current index scan API contract. Is there any thought behind
> exposing this state to callers instead of filtering stale entries
> internally?

This is a good question, I'll try to dive in and provide my thoughts which I don't pretend are the final/correct answer just where I am on this trade-off right now.

> I need to stop here to handle some other work. I will come back with
> more comments later.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

After much thought about the implicit contracts between the HEAP table AM, index AMs (mostly nbtree), and the executor I felt that I should try to untie a bit of the Gordian knot rather than add another thread to it. By that I mean I've tried hard in v68 to call out what was implicit before and make it explicit.

In this v68 series we have:

0001 - a table AM contract (or what I might like to call a "trait") similar to those found on indexes but the first of its kind for tables. This contract defines what a "locator" is, which in today's world is always a TID, and pull back into the table AM what enables a locator to do its job and interact with the executor and any number of index AMs.

0002 - this is a simple rename of "traversed" to retargeted in TM_FailureData to, in my mind, make it more clear what that boolean is communicating. Some might say this isn't required, and they'd be right, but I felt this change made it clearer to me and I hope others what this field means in practice so I've left it in this series.

0003 - now that we've defined a "locator" let's make it possible for the bytes that define the location to be a variable length so some table AM in the future (not introduced here in this series) could in theory store something other than a TID in an index that supports such behavior. Imagine a table that wants to provide the primary key as the "locator" (read: OrioleDB) or something somewhat TID-like but not entirely (see: Zedstore)

0004 - and while we're at it, let's stretch our minds a bit and consider that some table AMs might want to update in-place and manage MVCC differently from the model in HEAP. Again, nothing like that is introduced in this commit, but if we're going to define how an index and executor reference items stored in tables this feels like a case worth covering.

0005 - in this patch we get to the core issue that forced me to rethink and redesign Tepid a bit and bring to light the locator contract; the BitmapAnd/Or operations in the executor. Were it not for them this patch set could have sailed through months ago. But we have this nifty optimization whereby an index and executor can avoid pulling table (heap) pages under certain circumstances and so we need to continue to support that. To do that in the previous patch set I'd been co-opt'ing the 14th bit of a TID to signal a need to do something different when performing these bitmap scans, that left me feeling very dirty so I've found a new way to work around this which has different (hopefully lesser) trade-offs (you'll be the judge on that I'm sure). I had to find another method to address this issue head on. So, I've widened the VM map 2 bits to 4 bits in total (2x in size, say it isn't so!) and now track per-page in the VM with the third bit of those four if an indexes' entries for a row on the page may name different line pointers, which is the case where a bitmap AND scan must jump through hoops. BitmapOr turns out to be a non-issue for HEAP and TIDs, but there is room in the contract for table AMs that have other shapes to own that as well.

0006 - here is where we return to the earlier patch set which began by adding more tests as a way to baseline the existing HOT functionality more precisely than we've done so far, this commit is largely what it was before with some cleanup.

0007 - this is the commit that has traveled far from it's start in the thread on supporting expression indexes in the HOT update path, it is largely unchanged and still moves the logic of HeapDetermineColumnsInfo() from the HEAP into the executor while calling into the table AM for the AM's specific needs/layout so as not to leak HEAP-isms into the executor (well, not add more of those leaks).

0008 - here I borrow bits form the infomask for selective index updates (SIU) on the HOT path as in previous versions of these patches

0009 - this is the meat of the Tepid selective index updates (SIU) patch and the largest change set in this series, also mostly unchanged from before

0010 - update the prune/vacuum process to reclaim SIU chains

0011 - update statistics to include new useful metrics for observing SIU in action

0012 - update amcheck so it's not confused by the changes we've made thus far

0013 - enable some new choices on logical replication stemming from SIU

0014 - and the benchmark I'm using to measure for transparency sake, but not proposed for merge into core (just for you to tell me how I'm doing it wrong, lol)

And so there you have it, another in the long line of attempts at expanding the concepts and advantages of the HOT update path for HEAP but in a way that furthers other important goals as well. I hope you enjoy reading and reviewing this as much as I have enjoyed coding it. Also attached is my draft of a wiki page on this and you will also find a new README.HOT-INDEXED to (hopefully) shed some light on these changes. I have reviewed all of this code myself, I have at times also used LLMs to review and or adjust patches (split them, re-order them, etc.). This code will, I hope spark a debate on the merits of my approach to opening the door to a world where only modified indexes are updated, not ALL/NONE/SUMMARIZING, and I look forward to your thoughtful reviews and feedback on it.

best.

-greg

Attachment Content-Type Size
v68-tepid.tgz application/x-compressed-tar 213.7 KB
HOT-Indexed-Updates-Design.mediawiki application/octet-stream 55.6 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-09-28 02:54:22 Re: PSQL schema "describe" \dn is not escaping quotes
Previous Message Richard Guo 2026-09-28 01:01:39 Re: FDW RTE join pushdown fails to create plan with aggregates