| From: | shihao zhong <zhong950419(at)gmail(dot)com> |
|---|---|
| To: | Antonin Houska <ah(at)cybertec(dot)at>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: REPACK enhancements |
| Date: | 2026-09-15 04:20:35 |
| Message-ID: | CAGRkXqQoWEzT9C4p3CO-oKTXLjeyDG-RuoKHcQ5AUi8AAEhgcw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Antonin,
Alvaro suggested I review this thread. Starting with the preparatory
refactors 0001-0003, which stand on their own. They apply on ca326e903df
(needs a rebase on current master), build with --cassert, and the
injection_points/isolation and test_decoding suites pass.
0001: in heap_insert_for_repack(), the reform slot borrows the source
tuple's by-ref data (ExecForceStoreHeapTuple with shouldFree=false), but
'tuple' is freed before the insert reads from it -- a use-after-free if
ExecFetchSlotHeapTuple(src) returns shouldFree=true. Latent today (the copy
paths pass buffer/heaptuple slots), but the "nothing should depend on it
now" comment is wrong. Suggest freeing after the insert; that also drops the
slot-type assumption.
0002: pure code motion, LGTM.
0003: factors the per-destination fields of ChangeContext into a reusable
RepackDest, so 0004 can target the auxiliary relation as well as the new
heap.
Also, could you register this in the open commitfest so cfbot tracks it?
Testing 0004 now, will post new finding in following emails
Thanks,
Shihao
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-09-15 04:22:54 | Re: Add ASCII fast path to Unicode normalization functions |
| Previous Message | Ashutosh Bapat | 2026-09-15 04:15:21 | Re: Use-after-free of a shared Path in add_path()/add_partial_path() |