| From: | "Greg Burd" <greg(at)burd(dot)me> |
|---|---|
| To: | "Alexander Korotkov" <aekorotkov(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Tepid: selective index updates for heap relations |
| Date: | 2026-07-09 15:41:08 |
| Message-ID: | 97d5e5a3-f8e2-49ec-bd52-5b43e5d2b30a@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jul 9, 2026, at 7:26 AM, Alexander Korotkov wrote:
> Hi, Greg!
>
> Thank you for your work. It looks huge.
Thanks for looking at it, it is.
> On Tue, Jun 30, 2026 at 8:21 PM Greg Burd <greg(at)burd(dot)me> wrote:
>> Write path. heap_update gains a third mode (HEAP_SELECTIVE_INDEX_UPDATE)
>> alongside HEAP_UPDATE_ALL_INDEXES and HEAP_HEAP_ONLY_UPDATE. It keeps the
>> new tuple heap-only with its inline bitmap, and the executor inserts fresh
>> entries only into the indexes whose attributes changed (ExecSetIndexUnchanged);
>> the fresh entry points at the new heap-only tuple, not at the chain root.
>> If the page can't fit the new tuple, the update is downgraded to a normal
>> non-HOT update.
>
> I didn't yet read the whole design nor the whole patches. But this is
> the point that raises questions for me. How could this work with the
> present Bitmap Scans? I didn't find the direct answer in this thread.
> As you describe, entries to the indexes whose attributes changed are
> pointing to the new heap-only tuple (that effectively makes the tuple
> not heap-only, but that's just terminology question). For instance,
> you have old tuple T1, "new heap-only tuple" T2, unchanged index I1,
> and changed index I2. Both T1 and T2 have values V11 for I1. T1 and
> T2 have values V21 and V22 for I2 correspondingly. So, in this
> example according the paragraph above the logical contents of I1 is:
> V11 => T1, logical contents of I2 is V21 => T1, V22 => T2. Imagine,
> user searches for the tuple containing V11 for I1 and V22 for I2 with
> BitmapAnd. I1 side of BitmapAnd will return ctid of T1, while I2 side
> of BitmapAnd will return ctid of T2. That gives nothing in the
> intersection, while T2 is obviously matches the criteria.
You're right, and it's a real bug, not a theoretical one. I totally overlooked
it. I reproduced your exact scenario against the current tree:
CREATE TABLE t (id int PRIMARY KEY, c1 int, c2 int) WITH (fillfactor=50);
CREATE INDEX i1 ON t(c1); -- unchanged by the update
CREATE INDEX i2 ON t(c2); -- changed by the update
INSERT INTO t VALUES (1, 11, 21);
UPDATE t SET c2 = 22 WHERE id = 1; -- HOT-indexed: only c2/i2 changes
SET enable_indexscan = off; SET enable_seqscan = off; -- force BitmapAnd
SELECT id, c1, c2 FROM t WHERE c1 = 11 AND c2 = 22;
-- (0 rows) <-- wrong; (1, 11, 22) matches both
The leaf TIDs are exactly as you describe:
i1 (unchanged): c1=11 -> (0,1) [chain root]
i2 (changed): c2=21 -> (0,1), c2=22 -> (0,2) [fresh entry at new tuple]
So BitmapAnd intersects {(0,1)} with {(0,2)} and gets nothing. The heap
side of the scan does resolve both TIDs to the live tuple and does run the
recheck -- but that all happens *after* the TID-bitmap intersection, which
has already thrown the row away. A false negative, and Bitmap Scans can't
tolerate those.
I had this covered for plain index scans and index-only scans, where the
per-entry crossed-attribute recheck runs on the way to the heap, and I missed
that BitmapAnd makes its decision before any of that machinery gets a turn.
Thank you for catching it before it went any further.
> As you mention downthread, Bitmap Scan can tolerate false positives,
> but it can't tolerate false negatives. Bitmap Scan assumes CTIDs to
> be independently indexed pointers, and therefore corresponding bitmaps
> could be freely overlapped. I think Bitmap Scans was one of the
> critical weakness in WARM, and it is the question to be address in any
> further attempt in this direction.
I'm going to sit with it rather than fire back a half-formed fix; if you already
see why this is or isn't surmountable, I'd much rather hear that reasoning now.
> ------
> Regards,
> Alexander Korotkov
> Supabase
I'll publish v58 (rebased onto d3a10f3e128) and then hold the patch set where it
is until this is resolved -- no point polishing something with a hole in the
read path until I can plug it. I'll add a test case that captures this and
include it in future patches.
One other fix since v57: the macOS cfbot failure was a flaky regression test, not
a code bug. The hi_reclaim case in hot_indexed_updates.sql (via 002_pg_upgrade's
old-instance run) asserted a post-VACUUM state — chain collapsed to a redirect,
member count back to zero — but both of those outcomes depend on the deleted
tuple falling below the global xmin horizon, which a concurrent snapshot
elsewhere in the regression cluster can pin back indefinitely. So the test
passed or failed on timing. My v55-era attempt at this only traded one
horizon-dependent assertion for another; I confirmed the flakiness by
reproducing it under a held-open REPEATABLE READ snapshot.
The test now asserts only horizon-independent invariants: a HOT-indexed member
is always present (the live version can't be pruned), the live row still
resolves through the secondary index by its current key, and none of the
superseded keys surface. Collapse and snapshot-gated reclaim stay in the
hot_indexed_adversarial isolation spec, where transaction ordering — hence the
horizon — is controlled, and the collapse is validated by reader consistency
across it. Verified the new assertions hold under a deliberately held-back
horizon.
Thanks again; this is exactly the kind of review I was hoping the
design-questions-first post would draw out.
best.
-greg
| Attachment | Content-Type | Size |
|---|---|---|
| v58-0001-Add-tests-to-cover-a-variety-of-heap-HOT-update-.patch | text/x-patch | 45.4 KB |
| v58-0002-Identify-modified-indexed-attributes-in-the-exec.patch | text/x-patch | 61.4 KB |
| v58-0003-Add-the-HOT-indexed-on-disk-format-inline-attr-b.patch | text/x-patch | 12.8 KB |
| v58-0004-Add-HOT-indexed-updates-selective-index-maintena.patch | text/x-patch | 229.8 KB |
| v58-0005-Collapse-dead-HOT-indexed-chains-to-xid-free-stu.patch | text/x-patch | 54.1 KB |
| v58-0006-Teach-amcheck-to-recognize-HOT-indexed-chains-an.patch | text/x-patch | 18.0 KB |
| v58-0007-Add-HOT-indexed-statistics-and-the-comprehensive.patch | text/x-patch | 168.9 KB |
| v58-0008-Gate-HOT-indexed-updates-on-the-logical-replicat.patch | text/x-patch | 116.8 KB |
| v58-0009-DO-NOT-MERGE-Add-a-HOT-SIU-benchmark-harness.patch | text/x-patch | 33.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2026-07-09 15:52:12 | Re: libpq: Process buffered SSL read bytes to support records >8kB on async API |
| Previous Message | Peter Geoghegan | 2026-07-09 15:24:59 | Re: Hash index bucket split bug |