| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Alexander Lakhin <exclusion(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tomas(at)vondra(dot)me>, David Rowley <dgrowleyml(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Subject: | Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access) |
| Date: | 2026-09-14 14:57:44 |
| Message-ID: | CAAKRu_ZKgD3o920ed-d5Odxb7y9r60LjPGxsu1WW6UX0xczq4g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Sep 13, 2026 at 10:22 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > On Sep 12, 2026, at 04:08, Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
>
> > <v1-0001-Avoid-setting-pd_prune_xid-when-inserting-frozen-.patch>
>
> The change looks correct to me.
Thanks for the review. I've pushed this.
> One nit: perhaps we could take this opportunity to do a small refactoring, such as declaring i in the for loop and moving tbuf, htup, and newlen into the loop. This would reduce their scope and may improve readability.
>
> For your convenience, I made those small changes in v2. If you don’t like them, feel free to discard v2.
This was a good suggestion, but I prefer to keep bug fixes that get
backpatched as minimal as possible. Part of it is making it as
understandable as possible but also every single code change we make
can accidentally introduce bugs (I know these wouldn't) and the less
we can do that in backbranches the better. I would be happy to do this
kind of cleanup (reducing variable scope helps prevent bugs of course)
as part of a larger commit to master only, though.
Speaking of which, I noticed while working on this that 8e03eb92e9a,
which reverted setting the VM for single inserts, also made it so
inserting frozen rows into an already all-visible page cleared the VM
and PD_ALL_VISIBLE. I haven't studied it closely, but I think this
might just be an oversight and there is no reason to clear all-visible
when inserting only a frozen row. I'm not sure I would backpatch this,
but it seems reasonable to fix on master after some careful thought.
- Melanie
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David E. Wheeler | 2026-09-14 15:01:10 | Re: Add ASCII fast path to Unicode normalization functions |
| Previous Message | Andres Freund | 2026-09-14 14:56:12 | Re: ExecForceStoreHeapTuple() loses tts_tid, so ORDER BY-op index scans project an invalid ctid |