Re: maybe a type_sanity. sql bug

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Fri, Oct 27, 2023 at 11:45:44AM +0800, jian he wrote:
>> The test seems to assume the following sql query should return zero row.
>> but it does not. I don't know much about the "relreplident" column.

> The problem is about relkind, as 'I' refers to a partitioned index.
> That is a legal value in pg_class.relkind, but we forgot to list it in
> this test.

Yeah, in principle this check should allow any permissible relkind
value. In practice, because it runs so early in the regression tests,
there's not many values present. I added a quick check and found that
type_sanity only sees these values:

-- **************** pg_class ****************
-- Look for illegal values in pg_class fields
+select distinct relkind from pg_class order by 1;
+ relkind
+---------
+ i
+ r
+ t
+ v
+(4 rows)
+
SELECT c1.oid, c1.relname
FROM pg_class as c1
WHERE relkind NOT IN ('r', 'i', 'S', 't', 'v', 'm', 'c', 'f', 'p') OR

We've had some prior discussions about moving type_sanity, opr_sanity
etc to run later when there's a wider variety of objects present.
I'm not sure about that being a great idea though --- for example,
there's a test that creates an intentionally incomplete opclass
and even leaves it around for pg_dump stress testing. That'd
probably annoy opr_sanity if it ran after that one.

The original motivation for type_sanity and friends was mostly
to detect mistakes in the hand-rolled initial catalog contents,
and for that purpose it's fine if they run early. Some of what
they check is now redundant with genbki.pl I think.

Anyway, we should fix this if only for clarity's sake.
I do not feel a need to back-patch though.

regards, tom lane