On Thu, Mar 09, 2023 at 09:46:12AM +0900, Kyotaro Horiguchi wrote: > Although I'm not strongly opposed to pfreeing them, I'm not sure I > like the way the patch frees them. The life times of all of raw_data, > raw_page and flags are within a block. They can be freed > unconditionally after they are actually used and the scope of the > pointer variables can be properly narowed.
The thing is that you cannot keep them inside each individual blocks because they have to be freed once their values are stored in the tuplestore, which is why I guess Bharath has done things this way. After sleeping on that, I tend to prefer the simplicity of v3 where we keep track of the block and fpi data in each of their respective blocks. It means that we lose track of them each time we go to a different block, but the memory context reset done after each record means that scanning through a large WAL history will not cause a leak across the function call. The worst scenario with v3 is a record that makes use of all the 32 blocks with a hell lot of block data in each one of them, which is possible in theory, but very unlikely in practice except if someone uses a custom RGMR to generate crazily-shaped WAL records. I am aware of the fact that it is possible to generate such records if you are really willing to do so, aka this thread: https://www.postgresql.org/message-id/flat/CAEze2WgGiw+LZt+vHf8tWqB_6VxeLsMeoAuod0N=ij1q17n...@mail.gmail.com In short, my choice would still be simplicity here with v3, I guess. -- Michael
signature.asc
Description: PGP signature
