| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Hash index AM fake LSN oversight |
| Date: | 2026-07-15 06:47:33 |
| Message-ID: | CAON2xHPnBzvg5zRrrUZa97+7bWCP3tBzcMdH6068ROxZyTMwQQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Jul 15, 2026 at 6:48 AM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> I noticed a minor oversight in my commit e5836f7b: one remaining hash
> index WAL record type lacks a corresponding "PageSetLSN(buf,
> XLogGetFakeLSN(rel))" for the unlogged relation case.
Confirmed -- log_split_page() is the only data-page site e5836f7b left
without a fake-LSN branch (the _hash_init meta/bitmap paths set the LSN
under WAL only, but those aren't read concurrently, so that's expected).
The fix is correct.
One optional cosmetic point: the other fake-LSN sites, including the two
functions e5836f7b just reworked in this file, hoist recptr out of the
if and call PageSetLSN once afterwards, rather than once per branch.
Matching that form would keep log_split_page consistent with them:
log_split_page(Relation rel, Buffer buf)
{
XLogRecPtr recptr;
if (RelationNeedsWAL(rel))
{
XLogBeginInsert();
XLogRegisterBuffer(0, buf, REGBUF_FORCE_IMAGE | REGBUF_STANDARD);
recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_SPLIT_PAGE);
}
else
recptr = XLogGetFakeLSN(rel);
PageSetLSN(BufferGetPage(buf), recptr);
}
But your version is functionally identical with a smaller backpatch
diff, so this is entirely your call -- +1 either way.
>
> This is completely harmless, but it should be fixed on general
> principle. Attached patch fixes it.
>
> --
> Peter Geoghegan
--
Regards,
Ewan Young
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Devrim Gündüz | 2026-07-15 06:52:18 | Re: PG20 Minimum Dependency Thread |
| Previous Message | Amit Kapila | 2026-07-15 06:28:01 | Re: walsummarizer can get stuck when switching timelines |