Re: Fix for pageinspect bug in PG 17

On 11/12/24 08:39, Michael Paquier wrote:
> On Mon, Nov 11, 2024 at 07:32:10PM +0100, Tomas Vondra wrote:
>> This adds an out argument to brin_page_items, but I failed to consider
>> the user may still run with an older version of the extension - either
>> after pg_upgrade (as in the report), or when the CREATE EXTENSION
>> command specifies VERSION.
>
> Perhaps it would be worth adding a test where the function is called
> in the context of a past extension version? (I know, I'm noisy when
> it comes to that.)
>
>> The only fix I can think of is explicitly looking at TupleDesc->natts,
>> as in the attached fix.
>
> How about some consistency instead as this is the same error as
> 691e8b2e1889? btreefuncs.c is deciding to issue an error if we are
> trying to call one of its functions in the context of a past version
> of the extension. pageinspect is a superuser module, it does not seem
> that bad to me to tell users that they should force an upgrade when
> using it.

Yeah, a test with an older version is a good idea. And pageinspect
already has such tests in oldextversions.sql, so it was simple to just
add it there.

And I think you're right it's probably better to just error out. Yes, we
could check the natts value and build the right tuple, but if btree
functions already error out, it'd be a bit futile to do it differently
(although I'm not sure why bt_meta() says it can't be done reliably).

regards

--
Tomas Vondra