RE: Fix for pageinspect bug in PG 17

Dear Tomas,

> Here's a fix for pageinspect bug in PG17, reported in [1]. The bug turns
> out to be introduced by my commit

I could not see the link, but I think it is [1], right?

>
> commit dae761a87edae444d11a411f711f1d679bed5941
> Author: Tomas Vondra <tomas(dot)vondra(at)postgresql(dot)org>
> Date: Fri Dec 8 17:07:30 2023 +0100
>
> Add empty BRIN ranges during CREATE INDEX
>
> ...
>
> 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.

I confirmed that 428c0ca added new output entries (version is bumped 11->12),
and the same C function is used for both 11 and 12.

> The new "empty" field is added in the middle of the output tuple, which
> shifts the values, causing segfault when accessing a text field at the
> end of the array.

Agreed and I could reproduce by steps:

```
postgres=# CREATE EXTENSION pageinspect VERSION "1.11" ;
CREATE EXTENSION
postgres=# CREATE TABLE foo (id int);
CREATE TABLE
postgres=# CREATE INDEX ON foo USING brin ( id );
CREATE INDEX
postgres=# SELECT * FROM brin_page_items(get_raw_page('foo_id_idx', 2), 'foo_id_idx');
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```

>
> Of course, we add arguments to existing functions pretty often, and we
> know how to do that in backwards-compatible way - pg_stat_statements is
> a good example of how to do that nicely. Unfortunately, it's too late to
> do that for brin_page_items :-( There may already be upgraded systems or
> with installed pageinspect, etc.
>
> The only fix I can think of is explicitly looking at TupleDesc->natts,
> as in the attached fix.

I've tested your patch and it worked well. Also, I tried to upgrade from 11 and 12
and ran again, it could execute well.

Few points:

1.
Do you think we should follow the approach like pg_stat_statements for master?
Or, do you want to apply the patch both PG17 and HEAD? I think latter one is OK
because we should avoid code branches as much as possible.

2.
Later readers may surprise the part for checking `rsinfo->setDesc->natts`.
Can you use the boolean and add comments, something like

```
+ /* Support older API version */
+ bool output_empty_attr = (rsinfo->setDesc->natts >= 8);
```

3.
Do we have to add the test for the fix?

[1]: https://www.postgresql.org/message-id/VI1PR02MB63331C3D90E2104FD12399D38A5D2%40VI1PR02MB6333.eurprd02.prod.outlook.com

Best regards,
Hayato Kuroda
FUJITSU LIMITED