Re: Fix for pageinspect bug in PG 17

On Wed, Nov 13, 2024 at 09:00:30PM +0100, Tomas Vondra wrote:
> Does that mean you think we should fix the issue at hand differently?
> Say, by looking at number of columns and building the correct tuple,
> like I did in my initial patch?

691e8b2e18 is not something I would have done when it comes to
pageinspect, FWIW. There is the superuser argument for this module,
so I'd vote for an error and apply the same policy across all branches
as a matter of consistency.

A second argument in favor of raising an error is that it makes future
changes a bit easier to implement so as you can reuse the existing
routine as an "internal" workhorse called by the SQL functions
depending on the version of the extension involved at runtime, as we
do in pgss. Data type changes are easier to track, for example. If
you add a solution based on the number of arguments, you may have to
mix it with a per-version approach, at least on HEAD. As long as
there is a test to check on behavior or another, we're going to be
fine because we'll know if a previous assumption suddenly breaks.

Anyway, your solution to use the number of columns to determine what
should be done at runtime works as well, so feel free to just ignore
me. That's just an approach I'd prefer taking here because that's
easier to work with IMO, but I can also see why people don't like the
versioning logic a-la-PGSS, as well.
--
Michael