| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Greg Burd <greg(at)burd(dot)me> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Tepid: selective index updates for heap relations |
| Date: | 2026-07-17 20:12:43 |
| Message-ID: | CALj2ACV3YaHh4zxaOFkNqKeQzkX0-uW1ghRd4LwdZsOjRB1T4Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
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.
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).
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?
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?
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.
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?
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.
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)?
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.
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?
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
No need to specify test names in the comments, because they can change anytime.
* 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.
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.
I will continue reading the other patches in the coming weeks.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-07-17 20:15:44 | Re: Fix GROUP BY ALL handling of ORDER BY operator semantics |
| Previous Message | Bruce Momjian | 2026-07-17 20:04:28 | Re: First draft of PG 19 release notes |