pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: 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: Mon, 26 Aug 2024 12:26:27 -0500
Message-ID: <Zsy6w_yW2QKYSo_m@nathan> (raw)
In-Reply-To: <CACX+KaP3i+i9tdPLjF5JCHVv93xobEdcd_eB+638VDvZ3i=cQA@mail.gmail.com>
References: <CACX+KaP3i+i9tdPLjF5JCHVv93xobEdcd_eB+638VDvZ3i=cQA@mail.gmail.com>

On Mon, Aug 26, 2024 at 09:14:27PM +0530, Ayush Vatsa wrote:
> Given the situation, I see two potential paths forward:
> *1/ Reintroduce Support for Sequences in pgstattuple*: This would be a
> relatively small change. However, it's important to note that the purpose
> of pgstattuple is to provide statistics like the number of tuples, dead
> tuples, and free space in a relation. Sequences, on the other hand, return
> only one value at a time and don“t have attributes like dead tuples.
>
> [...] 
> 
> *2/ Explicitly Block Sequence Support in pgstattuple*: We could align
> sequences with other unsupported objects, such as foreign tables, by
> providing a more explicit error message.

While it is apparently pretty uncommon to use pgstattuple on sequences,
this is arguably a bug that should be fixed and back-patched.  I've CC'd
Michael Paquier, who is working on sequence AMs and may have thoughts.  I
haven't looked at his patch set for that, but I'm assuming that it might
fill in pg_class.relam for sequences, which would have the same effect as
option 1.

I see a couple of other places we might want to look into as part of this
thread.  Besides pgstattuple, I see that pageinspect and pg_surgery follow
a similar pattern.  pgrowlocks does, too, but that one seems intentionally
limited to RELKIND_RELATION.  I also see that amcheck explicitly allows
sequences:

	/*
	 * Sequences always use heap AM, but they don't show that in the catalogs.
	 * Other relkinds might be using a different AM, so check.
	 */
	if (ctx.rel->rd_rel->relkind != RELKIND_SEQUENCE &&
		ctx.rel->rd_rel->relam != HEAP_TABLE_AM_OID)
		ereport(ERROR,
				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
				 errmsg("only heap AM is supported")));

IMHO it would be good to establish some level of consistency here.

-- 
nathan





view thread (29+ messages)  latest in thread

Message-ID: <Zsy6w_yW2QKYSo_m@nathan>
Permalink:  ../Zsy6w_yW2QKYSo_m@nathan/
Also on:    postgresql.org/message-id/Zsy6w_yW2QKYSo_m@nathan

 · 

reply

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, ayushvatsa1810@gmail.com, michael@paquier.xyz
  Subject: Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch
  In-Reply-To: <Zsy6w_yW2QKYSo_m@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