diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index f35afddbea..31496a3063 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2465,7 +2465,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, * because the heaptuples data structure is all in local memory, not in * the shared buffer. */ - if (IsSystemRelation(relation)) + if (IsCatalogRelation(relation)) { for (i = 0; i < ntuples; i++) CacheInvalidateHeapTuple(relation, heaptuples[i], NULL); diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 06aa76627a..5a46fd963a 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -3628,7 +3628,7 @@ pg_class_aclmask(Oid table_oid, Oid roleid, * themselves. ACL_USAGE is if we ever have system sequences. */ if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | ACL_USAGE)) && - IsSystemClass(classForm) && + IsSystemClass(table_oid, classForm) && classForm->relkind != RELKIND_VIEW && !has_rolcatupdate(roleid) && !allowSystemTableMods) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 577206cd26..7719798457 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -97,22 +97,20 @@ GetDatabasePath(Oid dbNode, Oid spcNode) /* * IsSystemRelation - * True iff the relation is a system catalog relation. + * True iff the relation is either a system catalog or toast table. + * By a system catalog, we mean one that created in the pg_catalog schema + * during initdb. User-created relations in pg_catalog don't count as + * system catalogs. * * NB: TOAST relations are considered system relations by this test * for compatibility with the old IsSystemRelationName function. * This is appropriate in many places but not all. Where it's not, - * also check IsToastRelation. - * - * We now just test if the relation is in the system catalog namespace; - * so it's no longer necessary to forbid user relations from having - * names starting with pg_. + * also check IsToastRelation or use IsCatalogRelation(). */ bool IsSystemRelation(Relation relation) { - return IsSystemNamespace(RelationGetNamespace(relation)) || - IsToastNamespace(RelationGetNamespace(relation)); + return IsSystemClass(RelationGetRelid(relation), relation->rd_rel); } /* @@ -122,12 +120,60 @@ IsSystemRelation(Relation relation) * search pg_class directly. */ bool -IsSystemClass(Form_pg_class reltuple) +IsSystemClass(Oid relid, Form_pg_class reltuple) { - Oid relnamespace = reltuple->relnamespace; + return IsToastClass(reltuple) || IsCatalogClass(relid, reltuple); +} - return IsSystemNamespace(relnamespace) || - IsToastNamespace(relnamespace); +/* + * IsCatalogRelation + * True iff the relation is a system catalog, or the toast table for + * a system catalog. By a system catalog, we mean one that created + * in the pg_catalog schema during initdb. As with IsSystemRelation(), + * user-created relations in pg_catalog don't count as system catalogs. + * + * Note that IsSystemRelation() returns true for ALL toast relations, + * but this function returns true only for toast relations of system + * catalogs. + */ +bool +IsCatalogRelation(Relation relation) +{ + return IsCatalogClass(RelationGetRelid(relation), relation->rd_rel); +} + +/* + * IsCatalogClass + * True iff the relation is a system catalog relation. + * + * Check IsCatalogRelation() for details. + */ +bool +IsCatalogClass(Oid relid, Form_pg_class reltuple) +{ + Oid relnamespace = reltuple->relnamespace; + + /* + * Never consider relations outside pg_catalog/pg_toast to be catalog + * relations. + */ + if (!IsSystemNamespace(relnamespace) && !IsToastNamespace(relnamespace)) + return false; + + /* ---- + * Check whether the oid was assigned during initdb, when creating the + * initial template database. Minus the relations in information_schema + * excluded above, these are integral part of the system. + * We could instead check whether the relation is pinned in pg_depend, but + * this is noticeably cheaper and doesn't require catalog access. + * + * This test is safe since even a oid wraparound will preserve this + * property (c.f. GetNewObjectId()) and it has the advantage that it works + * correctly even if a user decides to create a relation in the pg_catalog + * namespace. + * ---- + */ + return relid < FirstNormalObjectId; } /* diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index a910f81666..6f2e1428eb 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -256,10 +256,17 @@ heap_create(const char *relname, Assert(OidIsValid(relid)); /* - * sanity checks + * Don't allow creating relations in pg_catalog directly, even though it + * is allowed to move user defined relations there. Semantics with search + * paths including pg_catalog are too confusing for now. + * + * But allow creating indexes on relations in pg_catalog even if + * allow_system_table_mods = off, upper layers already guarantee it's on a + * user defined relation, not a system one. */ if (!allow_system_table_mods && - (IsSystemNamespace(relnamespace) || IsToastNamespace(relnamespace)) && + ((IsSystemNamespace(relnamespace) && relkind != RELKIND_INDEX) || + IsToastNamespace(relnamespace)) && IsNormalProcessingMode()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index f6a5bfe8d1..afc6a78650 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1354,7 +1354,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, * ones the dependency changes would change. It's too late to be * making any data changes to the target catalog. */ - if (IsSystemClass(relform1)) + if (IsSystemClass(r1, relform1)) elog(ERROR, "cannot swap toast files by links for system catalogs"); /* Delete old dependencies */ diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2155252e4a..9b97e44867 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1824,6 +1824,7 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user) while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { Form_pg_class classtuple = (Form_pg_class) GETSTRUCT(tuple); + Oid relid = HeapTupleGetOid(tuple); if (classtuple->relkind != RELKIND_RELATION && classtuple->relkind != RELKIND_MATVIEW) @@ -1835,7 +1836,7 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user) continue; /* Check user/system classification, and optionally skip */ - if (IsSystemClass(classtuple)) + if (IsSystemClass(relid, classtuple)) { if (!do_system) continue; @@ -1850,7 +1851,7 @@ ReindexDatabase(const char *databaseName, bool do_system, bool do_user) continue; /* got it already */ old = MemoryContextSwitchTo(private_context); - relids = lappend_oid(relids, HeapTupleGetOid(tuple)); + relids = lappend_oid(relids, relid); MemoryContextSwitchTo(old); } heap_endscan(scan); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3483107e59..1aa1ad9127 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -910,7 +910,7 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid, aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, rel->relname); - if (!allowSystemTableMods && IsSystemClass(classform)) + if (!allowSystemTableMods && IsSystemClass(relOid, classform)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", @@ -2105,7 +2105,7 @@ renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing) if (!pg_class_ownercheck(myrelid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, NameStr(classform->relname)); - if (!allowSystemTableMods && IsSystemClass(classform)) + if (!allowSystemTableMods && IsSystemClass(myrelid, classform)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", @@ -10872,7 +10872,7 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, rv->relname); /* No system table modifications unless explicitly allowed. */ - if (!allowSystemTableMods && IsSystemClass(classform)) + if (!allowSystemTableMods && IsSystemClass(relid, classform)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index d86e9ad2c7..0008fc633b 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1174,7 +1174,7 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, /* you must own the table to rename one of its triggers */ if (!pg_class_ownercheck(relid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, rv->relname); - if (!allowSystemTableMods && IsSystemClass(form)) + if (!allowSystemTableMods && IsSystemClass(relid, form)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 954666ce04..de981cb372 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -128,7 +128,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, * Don't bother with indexes for an inheritance parent, either. */ if (inhparent || - (IgnoreSystemIndexes && IsSystemClass(relation->rd_rel))) + (IgnoreSystemIndexes && IsSystemRelation(relation))) hasindex = false; else hasindex = relation->rd_rel->relhasindex; diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index 2eca531749..b30902d2cd 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -858,7 +858,7 @@ RangeVarCallbackForRenameRule(const RangeVar *rv, Oid relid, Oid oldrelid, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is not a table or view", rv->relname))); - if (!allowSystemTableMods && IsSystemClass(form)) + if (!allowSystemTableMods && IsSystemClass(relid, form)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 5895102b51..7d75b3383f 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -110,7 +110,7 @@ CheckRelationOwnership(RangeVar *rel, bool noCatalogs) if (noCatalogs) { if (!allowSystemTableMods && - IsSystemClass((Form_pg_class) GETSTRUCT(tuple))) + IsSystemClass(relOid, (Form_pg_class) GETSTRUCT(tuple))) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c index bfe7d787b7..997a62f8ba 100644 --- a/src/backend/utils/cache/inval.c +++ b/src/backend/utils/cache/inval.c @@ -1040,15 +1040,15 @@ CacheInvalidateHeapTuple(Relation relation, /* * We only need to worry about invalidation for tuples that are in system - * relations; user-relation tuples are never in catcaches and can't affect + * catalogs; user-relation tuples are never in catcaches and can't affect * the relcache either. */ - if (!IsSystemRelation(relation)) + if (!IsCatalogRelation(relation)) return; /* - * TOAST tuples can likewise be ignored here. Note that TOAST tables are - * considered system relations so they are not filtered by the above test. + * IsCatalogRelation() will return true for TOAST tables of system + * catalogs, but we don't care about those, either. */ if (IsToastRelation(relation)) return; diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 44b6f38743..493493f997 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -25,9 +25,11 @@ extern char *GetDatabasePath(Oid dbNode, Oid spcNode); extern bool IsSystemRelation(Relation relation); extern bool IsToastRelation(Relation relation); +extern bool IsCatalogRelation(Relation relation); -extern bool IsSystemClass(Form_pg_class reltuple); +extern bool IsSystemClass(Oid relid, Form_pg_class reltuple); extern bool IsToastClass(Form_pg_class reltuple); +extern bool IsCatalogClass(Oid relid, Form_pg_class reltuple); extern bool IsSystemNamespace(Oid namespaceId); extern bool IsToastNamespace(Oid namespaceId); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 7366392457..232a233be5 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2337,3 +2337,37 @@ FROM ( 0 | t (1 row) +-- Checks on creating and manipulation of user defined relations in +-- pg_catalog. +-- +-- XXX: It would be useful to add checks around trying to manipulate +-- catalog tables, but that might have ugly consequences when run +-- against an existing server with allow_system_table_mods = on. +SHOW allow_system_table_mods; + allow_system_table_mods +------------------------- + off +(1 row) + +-- disallowed because of search_path issues with pg_dump +CREATE TABLE pg_catalog.new_system_table(); +ERROR: permission denied to create "pg_catalog.new_system_table" +DETAIL: System catalog modifications are currently disallowed. +-- instead create in public first, move to catalog +CREATE TABLE new_system_table(id serial primary key, othercol text); +ALTER TABLE new_system_table SET SCHEMA pg_catalog; +-- XXX: it's currently impossible to move relations out of pg_catalog +ALTER TABLE new_system_table SET SCHEMA public; +ERROR: cannot remove dependency on schema pg_catalog because it is a system object +-- move back, will currently error out, already there +ALTER TABLE new_system_table SET SCHEMA pg_catalog; +ERROR: table new_system_table is already in schema "pg_catalog" +ALTER TABLE new_system_table RENAME TO old_system_table; +CREATE INDEX old_system_table__othercol ON old_system_table (othercol); +INSERT INTO old_system_table(othercol) VALUES ('somedata'), ('otherdata'); +UPDATE old_system_table SET id = -id; +DELETE FROM old_system_table WHERE othercol = 'somedata'; +TRUNCATE old_system_table; +ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey; +ALTER TABLE old_system_table DROP COLUMN othercol; +DROP TABLE old_system_table; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 0d3b79bfcf..b55543076f 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1567,3 +1567,31 @@ FROM ( FROM pg_class WHERE relkind IN ('r', 'i', 'S', 't', 'm') ) mapped; + +-- Checks on creating and manipulation of user defined relations in +-- pg_catalog. +-- +-- XXX: It would be useful to add checks around trying to manipulate +-- catalog tables, but that might have ugly consequences when run +-- against an existing server with allow_system_table_mods = on. + +SHOW allow_system_table_mods; +-- disallowed because of search_path issues with pg_dump +CREATE TABLE pg_catalog.new_system_table(); +-- instead create in public first, move to catalog +CREATE TABLE new_system_table(id serial primary key, othercol text); +ALTER TABLE new_system_table SET SCHEMA pg_catalog; + +-- XXX: it's currently impossible to move relations out of pg_catalog +ALTER TABLE new_system_table SET SCHEMA public; +-- move back, will currently error out, already there +ALTER TABLE new_system_table SET SCHEMA pg_catalog; +ALTER TABLE new_system_table RENAME TO old_system_table; +CREATE INDEX old_system_table__othercol ON old_system_table (othercol); +INSERT INTO old_system_table(othercol) VALUES ('somedata'), ('otherdata'); +UPDATE old_system_table SET id = -id; +DELETE FROM old_system_table WHERE othercol = 'somedata'; +TRUNCATE old_system_table; +ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey; +ALTER TABLE old_system_table DROP COLUMN othercol; +DROP TABLE old_system_table;