public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Melanie Plageman <[email protected]>
To: Tom Lane <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: Small omission in type_sanity.sql
Date: Fri, 27 Jan 2023 17:25:09 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_ZZnN7XBDsWwEDyE1X=y3xs7GX-L=ftDk9gUg_4sX_awg@mail.gmail.com>
References: <CAAKRu_ZZnN7XBDsWwEDyE1X=y3xs7GX-L=ftDk9gUg_4sX_awg@mail.gmail.com>

Hi,

On 2023-01-18 14:51:32 -0500, Melanie Plageman wrote:
> I only changed these few lines in type_sanity to be more correct; I
> didn't change anything else in regress to actually exercise them (e.g.
> ensuring a partitioned table is around when running type_sanity). It
> might be worth moving type_sanity down in the parallel schedule?

Unfortunately it's not entirely trivial to do that. Despite the comment at the
top of type_sanity.sql:
-- None of the SELECTs here should ever find any matching entries,
-- so the expected output is easy to maintain ;-).

there are actually a few queries in there that are expected to return
objects. And would return more at the end of the tests.

That doesn't seem great.


Tom, is there a reason we run the various sanity tests early-ish in the
schedule? It does seem to reduce their effectiveness a bit...


Problems:
- shell types show up in a bunch of queries, e.g. "Look for illegal values in
  pg_type fields." due to NOT t1.typisdefined
- the omission of various relkinds noted by Melanie shows in "Look for illegal
  values in pg_class fields"
- pg_attribute query doesn't know about dropped columns
- "Cross-check against pg_type entry" is far too strict about legal combinations
  of typstorage



> It does seem a bit hard to remember to update these tests in
> type_sanity.sql when adding some new value for a pg_class field. I
> wonder if there is a better way of testing this.

As evidenced by the above list of failures, moving the test to the end of the
regression tests would help, if we can get there.



> --- All tables and indexes should have an access method.
> -SELECT c1.oid, c1.relname
> -FROM pg_class as c1
> -WHERE c1.relkind NOT IN ('S', 'v', 'f', 'c') and
> -    c1.relam = 0;
> - oid | relname 
> ------+---------
> +-- All tables and indexes except partitioned tables should have an access
> +-- method.
> +SELECT oid, relname, relkind, relam
> +FROM pg_class
> +WHERE relkind NOT IN ('S', 'v', 'f', 'c', 'p') and
> +    relam = 0;
> + oid | relname | relkind | relam 
> +-----+---------+---------+-------
>  (0 rows)

Don't think that one is right, a partitioned table doesn't have an AM.


> --- Conversely, sequences, views, types shouldn't have them
> -SELECT c1.oid, c1.relname
> -FROM pg_class as c1
> -WHERE c1.relkind IN ('S', 'v', 'f', 'c') and
> -    c1.relam != 0;
> - oid | relname 
> ------+---------
> +-- Conversely, sequences, views, types, and partitioned tables shouldn't have
> +-- them
> +SELECT oid, relname, relkind, relam
> +FROM pg_class
> +WHERE relkind IN ('S', 'v', 'f', 'c', 'p') and
> +    relam != 0;
> + oid | relname | relkind | relam 
> +-----+---------+---------+-------
>  (0 rows)

Particularly because you include them again here :)


Greetings,

Andres Freund

view thread (2+ messages)

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Small omission in type_sanity.sql
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox