Re: Snapshot leak warning with lo_export in subtransaction

On 27/09/2021 15:33, Alvaro Herrera wrote:
> On 2021-Sep-27, Heikki Linnakangas wrote:
>
>> This should be backpatched to all supported versions. This adds an argument
>> to 'inv_open' function, but I don't think there are extensions that use the
>> inv_*() functions directly. inv_api.c relies on close_lo_relation() being
>> called at the end of transaction, so I think an extension would find it hard
>> to use those functions correctly, anyway.
>
> Hmm, but you could also use a new value in its existing 'flags' argument
> instead of changing ABI.

I tried that, but didn't like the result. It conflated the user-visible
INV_READ/WRITE flags with the new internal-only flag.

Thinking about this some more, I came up with the attached. It moves the
responsibility of registering the snapshot from inv_api.c to the caller.
With that change, there's no need for a new option to inv_open(). The
division of labor between be-fsstubs.c and inv_api.c has always been a
bit blurry, I think that this makes it slightly more clear.

With this change, if there is an existing extension out there that calls
inv_open(), the returned desc will be short-lived. As I said above, I
think that's a reasonable assumption because dealing with a long-lived
LO desc correctly would require the caller to jump through some hoops
even before this change.

- Heikki