Обсуждение: Re: pgsql: pageinspect: Fix types used for bt_metap() columns.

On 2020-Mar-08, Peter Geoghegan wrote:

> Fix these issues by changing the declaration of bt_metap() to
> consistently use data types that can reliably represent all possible
> values.  This fixes things on HEAD only.  No backpatch, since it doesn't
> seem like there is a safe way to fix the issue without including a new
> version of the pageinspect extension (HEAD/Postgres 13 already
> introduced a new version of the extension).  Besides, the oldest_xact
> issue has been around since the release of Postgres 11, and we haven't
> heard any complaints about it before now.

This may be a good time to think through about how to implement a
version history for an extension that enables users to go from pg12's
current 1.7 pageinspect to a new fixed version in pg12, say 1.7.1, and
in HEAD provide an upgrade path from both 1.7 and 1.7.1 to master's 1.8.
Then you can pg_upgrade from pg12 to pg13 having either 1.7 or 1.7.1,
and you will be able to get to 1.8 nonetheless.

Does that make sense?

The current problem might not be serious enough to warrant actually
writing the code that would be needed, but I propose to at least think
about it.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Mar 9, 2020 at 8:55 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> This may be a good time to think through about how to implement a
> version history for an extension that enables users to go from pg12's
> current 1.7 pageinspect to a new fixed version in pg12, say 1.7.1, and
> in HEAD provide an upgrade path from both 1.7 and 1.7.1 to master's 1.8.
> Then you can pg_upgrade from pg12 to pg13 having either 1.7 or 1.7.1,
> and you will be able to get to 1.8 nonetheless.
>
> Does that make sense?

Sort of. The main problem with that idea is that it requires the user
to notice that the problem is in the extension definition itself,
which is a pretty rare edge case -- how many people will follow
through with that? You could deliberately break it so they'd have to
notice it, which is what I did here, but I don't think that users
would appreciate seeing that in a point release. Especially for
something like bt_metap(), which kind of worked before.

There are also implementation problems. You might need rather a lot of
upgrade paths. While the most significant problem by far here is with
the oldest_xact column, this bug was in the earliest version of the
pageinspect extension. Even 1.0 uses int4 where it should use
something that works as BlockNumber (I used int8 for this here).
That's just messy.

> The current problem might not be serious enough to warrant actually
> writing the code that would be needed, but I propose to at least think
> about it.

I briefly considered doing something like targeting the backbranches
by making oldest_xact display XIDs greater than 2^31-1 as negative
values, or as NULL, with a NOTICE message. I quickly concluded that
the cure would be worse than the disease.

-- 
Peter Geoghegan