Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch

On Fri, 30 Aug 2024, 23:06 Nathan Bossart, <nathandbossart(at)gmail(dot)com> wrote:
>
> On Fri, Aug 30, 2024 at 04:07:30PM -0400, Robert Haas wrote:
> > On Thu, Aug 29, 2024 at 1:36 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> >> Thanks. Robert, do you have any concerns with this?
> >
> > I don't know if I'm exactly concerned but I don't understand what
> > problem we're solving, either. I thought Ayush said that the function
> > wouldn't produce useful results for sequences; so then why do we need
> > to change the code to enable it?
>
> I suppose it would be difficult to argue that it is actually useful, given
> it hasn't worked since v11 and apparently nobody noticed until recently.
> If we're content to leave it unsupported, then sure, let's just remove the
> "relkind == RELKIND_SEQUENCE" check in pgstat_relation(). But I also don't
> have a great reason to _not_ support it. It used to work (which appears to
> have been intentional, based on the code), it was unintentionally broken,
> and it'd work again with a ~1 line change. "SELECT count(*) FROM
> my_sequence" probably doesn't provide a lot of value, but I have no
> intention of proposing a patch that removes support for that.
>
> All that being said, I don't have a terribly strong opinion, but I guess I
> lean towards re-enabling.
>
> Another related inconsistency I just noticed in pageinspect:
>
> postgres=# select t_data from heap_page_items(get_raw_page('s', 0));
> t_data
> --------------------------------------
> \x0100000000000000000000000000000000
> (1 row)
>
> postgres=# select tuple_data_split('s'::regclass, t_data, t_infomask, t_infomask2, t_bits) from heap_page_items(get_raw_page('s', 0));
> ERROR: only heap AM is supported

I don't think this is an inconsistency:

heap_page_items works on a raw page-as-bytea (produced by
get_raw_page) without knowing about or accessing the actual relation
type of that page, so it doesn't have the context why it should error
out if the page looks similar enough to a heap page. I could feed it
an arbitrary bytea, and it should still work as long as that bytea
looks similar enough to a heap page.
tuple_data_split, however, uses the regclass to decode the contents of
the tuple, and can thus determine with certainty based on that
regclass that it was supplied incorrect (non-heapAM table's regclass)
arguments. It therefore has enough context to bail out and stop trying
to decode the page's tuple data.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)