On Wed, Aug 16, 2023 at 4:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Makes sense.
Thanks for looking!
> If you change smgrclose() to do what smgrrelease() does now, then it
> will apply automatically to extensions.
>
> If an extension is currently using smgropen()/smgrclose() correctly,
> this patch alone won't make it wrong, so it's not very critical for
> extensions to adopt the change. However, if after this we consider it OK
> to hold a pointer to SMgrRelation for longer, and start doing that in
> the backend, then extensions need to be adapted too.
Yeah, that sounds quite compelling. Let's try that way:
* smgrrelease() is removed
* smgrclose() now releases resources, but doesn't destroy until EOX
* smgrdestroy() now frees memory, and should rarely be used
Still WIP while I think about edge cases, but so far I think this is
the better option.
> > While studying this I noticed a minor thinko in smgrrelease() in
> > 15+16, so here's a fix for that also. I haven't figured out a
> > sequence that makes anything bad happen, but we should really
> > invalidate smgr_targblock when a relfilenode is reused, since it might
> > point past the end. This becomes more obvious once smgrrelease() is
> > used for truncation, as proposed here.
>
> +1. You can move the smgr_targblock clearing out of the loop, though.
Right, thanks. Pushed.