From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Ayush Vatsa <ayushvatsa1810@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Cc: michael@paquier.xyz
Subject: Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch
Date: Fri, 30 Aug 2024 16:06:03 -0500
Message-ID: <ZtI0O-bN90P02oQO@nathan> (raw)
In-Reply-To: <CA+TgmoaHX9BYrr1u9c356ovGwVBy25Q9y_LdSYS8JW0pqd18JA@mail.gmail.com>
References: <CACX+KaP3i+i9tdPLjF5JCHVv93xobEdcd_eB+638VDvZ3i=cQA@mail.gmail.com>
<Zsy6w_yW2QKYSo_m@nathan>
<CA+TgmoYm0X=KuHry1A6prNzf3pUmAcXYgQEr25kCY8jSMJaVjg@mail.gmail.com>
<ZszGBMK1D4hCsRnD@nathan>
<CACX+KaNhg-ZwkqCSjsq7j_gRYDmHip2b4kwa6ohac0MspSeW9w@mail.gmail.com>
<ZtCxo95K_bqGbmNQ@nathan>
<CA+TgmoaHX9BYrr1u9c356ovGwVBy25Q9y_LdSYS8JW0pqd18JA@mail.gmail.com>
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@gmail.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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, robertmhaas@gmail.com, ayushvatsa1810@gmail.com, michael@paquier.xyz
Subject: Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch
In-Reply-To: <ZtI0O-bN90P02oQO@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox