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

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Ayush Vatsa <ayushvatsa1810(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, michael(at)paquier(dot)xyz
Subject: Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch
Date: 2024-08-30 21:06:03
Message-ID: ZtI0O-bN90P02oQO@nathan
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

--
nathan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jelte Fennema-Nio 2024-08-30 21:24:58 Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Previous Message Tom Lane 2024-08-30 20:55:09 Re: [EXTERNAL] Re: Add non-blocking version of PQcancel