Add testing to detect errors of omission in "pin" dependency creation.

It's essential that initdb.c's setup_depend() scan each system catalog
that could contain objects that need to have "p" (pin) entries in pg_depend
or pg_shdepend.  Forgetting to add that, either when a catalog is first
invented or when it first acquires DATA() entries, is an obvious bug
hazard.  We can detect such omissions at reasonable cost by probing every
OID-containing system catalog to see whether the lowest-numbered OID in it
is pinned.  If so, the catalog must have been properly accounted for in
setup_depend().  If the lowest OID is above FirstNormalObjectId then the
catalog must have been empty at the end of initdb, so it doesn't matter.
There are a small number of catalogs whose first entry is made later in
initdb than setup_depend(), resulting in nonempty expected output of the
test, but these can be manually inspected to see that they are OK.  Any
future mistake of this ilk will manifest as a new entry in the test's
output.

Since pg_conversion is already in the test's output, add it to the set of
catalogs scanned by setup_depend().  That has no effect today (hence, no
catversion bump here) but it will protect us if we ever do add pin-worthy
conversions.

This test is very much like the catalog sanity checks embodied in
opr_sanity.sql and type_sanity.sql, but testing pg_depend doesn't seem to
fit naturally into either of those scripts' charters.  Hence, invent a new
test script misc_sanity.sql, which can be a home for this as well as tests
on any other catalogs we might want in future.

Discussion: https://postgr.es/m/8068.1498155068@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2017-06-23 11:03:04 -04:00
parent da2322883b
commit 8be8510cf8
5 changed files with 171 additions and 4 deletions

View file

@ -1478,9 +1478,16 @@ setup_depend(FILE *cmdfd)
* for instance) but generating only the minimum required set of
* dependencies seems hard.
*
* Note that we deliberately do not pin the system views, which
* haven't been created yet. Also, no conversions, databases, or
* tablespaces are pinned.
* Catalogs that are intentionally not scanned here are:
*
* pg_database: it's a feature, not a bug, that template1 is not
* pinned.
*
* pg_extension: a pinned extension isn't really an extension, hmm?
*
* pg_tablespace: tablespaces don't participate in the dependency
* code, and DropTableSpace() explicitly protects the built-in
* tablespaces.
*
* First delete any already-made entries; PINs override all else, and
* must be the only entries for their objects.
@ -1501,6 +1508,8 @@ setup_depend(FILE *cmdfd)
"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
" FROM pg_constraint;\n\n",
"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
" FROM pg_conversion;\n\n",
"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
" FROM pg_attrdef;\n\n",
"INSERT INTO pg_depend SELECT 0,0,0, tableoid,oid,0, 'p' "
" FROM pg_language;\n\n",
@ -2906,6 +2915,11 @@ initialize_data_directory(void)
setup_depend(cmdfd);
/*
* Note that no objects created after setup_depend() will be "pinned".
* They are all droppable at the whim of the DBA.
*/
setup_sysviews(cmdfd);
setup_description(cmdfd);

View file

@ -0,0 +1,78 @@
--
-- MISC_SANITY
-- Sanity checks for common errors in making system tables that don't fit
-- comfortably into either opr_sanity or type_sanity.
--
-- Every test failure in this file should be closely inspected.
-- The description of the failing test should be read carefully before
-- adjusting the expected output. In most cases, the queries should
-- not find *any* matching entries.
--
-- NB: run this test early, because some later tests create bogus entries.
-- **************** pg_depend ****************
-- Look for illegal values in pg_depend fields.
-- classid/objid can be zero, but only in 'p' entries
SELECT *
FROM pg_depend as d1
WHERE refclassid = 0 OR refobjid = 0 OR
deptype NOT IN ('a', 'e', 'i', 'n', 'p') OR
(deptype != 'p' AND (classid = 0 OR objid = 0)) OR
(deptype = 'p' AND (classid != 0 OR objid != 0 OR objsubid != 0));
classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
---------+-------+----------+------------+----------+-------------+---------
(0 rows)
-- **************** pg_shdepend ****************
-- Look for illegal values in pg_shdepend fields.
-- classid/objid can be zero, but only in 'p' entries
SELECT *
FROM pg_shdepend as d1
WHERE refclassid = 0 OR refobjid = 0 OR
deptype NOT IN ('a', 'o', 'p', 'r') OR
(deptype != 'p' AND (dbid = 0 OR classid = 0 OR objid = 0)) OR
(deptype = 'p' AND (dbid != 0 OR classid != 0 OR objid != 0 OR objsubid != 0));
dbid | classid | objid | objsubid | refclassid | refobjid | deptype
------+---------+-------+----------+------------+----------+---------
(0 rows)
-- Check each OID-containing system catalog to see if its lowest-numbered OID
-- is pinned. If not, and if that OID was generated during initdb, then
-- perhaps initdb forgot to scan that catalog for pinnable entries.
-- Generally, it's okay for a catalog to be listed in the output of this
-- test if that catalog is scanned by initdb.c's setup_depend() function;
-- whatever OID the test is complaining about must have been added later
-- in initdb, where it intentionally isn't pinned. Legitimate exceptions
-- to that rule are listed in the comments in setup_depend().
do $$
declare relnm text;
reloid oid;
shared bool;
lowoid oid;
pinned bool;
begin
for relnm, reloid, shared in
select relname, oid, relisshared from pg_class
where relhasoids and oid < 16384 order by 1
loop
execute 'select min(oid) from ' || relnm into lowoid;
continue when lowoid is null or lowoid >= 16384;
if shared then
pinned := exists(select 1 from pg_shdepend
where refclassid = reloid and refobjid = lowoid
and deptype = 'p');
else
pinned := exists(select 1 from pg_depend
where refclassid = reloid and refobjid = lowoid
and deptype = 'p');
end if;
if not pinned then
raise notice '% contains unpinned initdb-created object(s)', relnm;
end if;
end loop;
end$$;
NOTICE: pg_constraint contains unpinned initdb-created object(s)
NOTICE: pg_conversion contains unpinned initdb-created object(s)
NOTICE: pg_database contains unpinned initdb-created object(s)
NOTICE: pg_extension contains unpinned initdb-created object(s)
NOTICE: pg_rewrite contains unpinned initdb-created object(s)
NOTICE: pg_tablespace contains unpinned initdb-created object(s)

View file

@ -30,7 +30,7 @@ test: point lseg line box path polygon circle date time timetz timestamp timesta
# geometry depends on point, lseg, box, path, polygon and circle
# horology depends on interval, timetz, timestamp, timestamptz, reltime and abstime
# ----------
test: geometry horology regex oidjoins type_sanity opr_sanity comments expressions
test: geometry horology regex oidjoins type_sanity opr_sanity misc_sanity comments expressions
# ----------
# These four each depend on the previous one

View file

@ -49,6 +49,7 @@ test: regex
test: oidjoins
test: type_sanity
test: opr_sanity
test: misc_sanity
test: comments
test: expressions
test: insert

View file

@ -0,0 +1,74 @@
--
-- MISC_SANITY
-- Sanity checks for common errors in making system tables that don't fit
-- comfortably into either opr_sanity or type_sanity.
--
-- Every test failure in this file should be closely inspected.
-- The description of the failing test should be read carefully before
-- adjusting the expected output. In most cases, the queries should
-- not find *any* matching entries.
--
-- NB: run this test early, because some later tests create bogus entries.
-- **************** pg_depend ****************
-- Look for illegal values in pg_depend fields.
-- classid/objid can be zero, but only in 'p' entries
SELECT *
FROM pg_depend as d1
WHERE refclassid = 0 OR refobjid = 0 OR
deptype NOT IN ('a', 'e', 'i', 'n', 'p') OR
(deptype != 'p' AND (classid = 0 OR objid = 0)) OR
(deptype = 'p' AND (classid != 0 OR objid != 0 OR objsubid != 0));
-- **************** pg_shdepend ****************
-- Look for illegal values in pg_shdepend fields.
-- classid/objid can be zero, but only in 'p' entries
SELECT *
FROM pg_shdepend as d1
WHERE refclassid = 0 OR refobjid = 0 OR
deptype NOT IN ('a', 'o', 'p', 'r') OR
(deptype != 'p' AND (dbid = 0 OR classid = 0 OR objid = 0)) OR
(deptype = 'p' AND (dbid != 0 OR classid != 0 OR objid != 0 OR objsubid != 0));
-- Check each OID-containing system catalog to see if its lowest-numbered OID
-- is pinned. If not, and if that OID was generated during initdb, then
-- perhaps initdb forgot to scan that catalog for pinnable entries.
-- Generally, it's okay for a catalog to be listed in the output of this
-- test if that catalog is scanned by initdb.c's setup_depend() function;
-- whatever OID the test is complaining about must have been added later
-- in initdb, where it intentionally isn't pinned. Legitimate exceptions
-- to that rule are listed in the comments in setup_depend().
do $$
declare relnm text;
reloid oid;
shared bool;
lowoid oid;
pinned bool;
begin
for relnm, reloid, shared in
select relname, oid, relisshared from pg_class
where relhasoids and oid < 16384 order by 1
loop
execute 'select min(oid) from ' || relnm into lowoid;
continue when lowoid is null or lowoid >= 16384;
if shared then
pinned := exists(select 1 from pg_shdepend
where refclassid = reloid and refobjid = lowoid
and deptype = 'p');
else
pinned := exists(select 1 from pg_depend
where refclassid = reloid and refobjid = lowoid
and deptype = 'p');
end if;
if not pinned then
raise notice '% contains unpinned initdb-created object(s)', relnm;
end if;
end loop;
end$$;