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