database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-22 02:52:08 +03:00
Code Activity

Clean up the behavior and API of catalog.c's is-catalog-relation tests.

Browse Source 
The right way for IsCatalogRelation/Class to behave is to return true
for OIDs less than FirstBootstrapObjectId (not FirstNormalObjectId),
without any of the ad-hoc fooling around with schema membership.

The previous code was wrong because (1) it claimed that
information_schema tables were not catalog relations but their toast
tables were, which is silly; and (2) if you dropped and recreated
information_schema, which is a supported operation, the behavior
changed.  That's even sillier.  With this definition, "catalog
relations" are exactly the ones traceable to the postgres.bki data,
which seems like what we want.

With this simplification, we don't actually need access to the pg_class
tuple to identify a catalog relation; we only need its OID.  Hence,
replace IsCatalogClass with "IsCatalogRelationOid(oid)".  But keep
IsCatalogRelation as a convenience function.

This allows fixing some arguably-wrong semantics in contrib/sepgsql and
ReindexRelationConcurrently, which were using an IsSystemNamespace test
where what they really should be using is IsCatalogRelationOid.  The
previous coding failed to protect toast tables of system catalogs, and
also was not on board with the general principle that user-created tables
do not become catalogs just by virtue of being renamed into pg_catalog.
We can also get rid of a messy hack in ReindexMultipleTables.

While we're at it, also rename IsSystemNamespace to IsCatalogNamespace,
because the previous name invited confusion with the more expansive
semantics used by IsSystemRelation/Class.

Also improve the comments in catalog.c.

There are a few remaining places in replication-related code that are
special-casing OIDs below FirstNormalObjectId.  I'm inclined to think
those are wrong too, and if there should be any special case it should
just extend to FirstBootstrapObjectId.  But first we need to debate
whether a FOR ALL TABLES publication should include information_schema.

Discussion: https://postgr.es/m/21697.1557092753@sss.pgh.pa.us
Discussion: https://postgr.es/m/15150.1557257111@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2019-05-08 23:27:29 -04:00
parent 3ae3c18b36
commit 2d7d946cd3
9 changed files with 81 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/backend/commands/indexcmds.c
View File

@ -2590,15 +2590,11 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue;
/*
* Skip system tables that index_create() would reject to index
* concurrently. XXX We need the additional check for
* FirstNormalObjectId to skip information_schema tables, because
* IsCatalogClass() here does not cover information_schema, but the
* check in index_create() will error on the TOAST tables of
* information_schema tables.
* Skip system tables, since index_create() would reject indexing them
* concurrently (and it would likely fail if we tried).
*/
if (concurrent &&
(IsCatalogClass(relid, classtuple) || relid < FirstNormalObjectId))
IsCatalogRelationOid(relid))
{
if (!concurrent_warning)
ereport(WARNING,
@ -2842,7 +2838,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
errmsg("concurrent reindex is not supported for shared relations")));
/* A system catalog cannot be reindexed concurrently */
if (IsSystemNamespace(get_rel_namespace(heapId)))
if (IsCatalogRelationOid(heapId))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("concurrent reindex is not supported for catalog relations")));