Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2d7d946

Browse files
committed
Clean up the behavior and API of catalog.c's is-catalog-relation tests.
The right way for IsCatalogRelation/Class to behave is to return truefor 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 thatinformation_schema tables were not catalog relations but their toasttables were, which is silly; and (2) if you dropped and recreatedinformation_schema, which is a supported operation, the behaviorchanged. That's even sillier. With this definition, "catalogrelations" 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_classtuple to identify a catalog relation; we only need its OID. Hence,replace IsCatalogClass with "IsCatalogRelationOid(oid)". But keepIsCatalogRelation as a convenience function.This allows fixing some arguably-wrong semantics in contrib/sepgsql andReindexRelationConcurrently, which were using an IsSystemNamespace testwhere what they really should be using is IsCatalogRelationOid. Theprevious coding failed to protect toast tables of system catalogs, andalso was not on board with the general principle that user-created tablesdo 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 expansivesemantics used by IsSystemRelation/Class.Also improve the comments in catalog.c.There are a few remaining places in replication-related code that arespecial-casing OIDs below FirstNormalObjectId. I'm inclined to thinkthose are wrong too, and if there should be any special case it shouldjust extend to FirstBootstrapObjectId. But first we need to debatewhether a FOR ALL TABLES publication should include information_schema.Discussion:https://postgr.es/m/21697.1557092753@sss.pgh.pa.usDiscussion:https://postgr.es/m/15150.1557257111@sss.pgh.pa.us
1 parent3ae3c18 commit2d7d946

File tree

9 files changed

+81
-67
lines changed

9 files changed

+81
-67
lines changed

‎contrib/sepgsql/dml.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,10 @@ check_relation_privileges(Oid relOid,
161161
*/
162162
if (sepgsql_getenforce()>0)
163163
{
164-
Oidrelnamespace=get_rel_namespace(relOid);
165-
166-
if (IsSystemNamespace(relnamespace)&&
167-
(required& (SEPG_DB_TABLE__UPDATE |
164+
if ((required& (SEPG_DB_TABLE__UPDATE |
168165
SEPG_DB_TABLE__INSERT |
169-
SEPG_DB_TABLE__DELETE))!=0)
166+
SEPG_DB_TABLE__DELETE))!=0&&
167+
IsCatalogRelationOid(relOid))
170168
ereport(ERROR,
171169
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
172170
errmsg("SELinux: hardwired security policy violation")));

‎src/backend/access/transam/varsup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ GetNewObjectId(void)
520520
* Check for wraparound of the OID counter. We *must* not return 0
521521
* (InvalidOid), and in normal operation we mustn't return anything below
522522
* FirstNormalObjectId since that range is reserved for initdb (see
523-
*IsCatalogClass()). Note we are relying on unsigned comparison.
523+
*IsCatalogRelationOid()). Note we are relying on unsigned comparison.
524524
*
525525
* During initdb, we start the OID generator at FirstBootstrapObjectId, so
526526
* we only wrap if before that point when in bootstrap or standalone mode.

‎src/backend/catalog/catalog.c

Lines changed: 59 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,18 @@
5353

5454
/*
5555
* IsSystemRelation
56-
*True iff the relation is either a system catalog or toast table.
57-
*By a system catalog, we mean one that created in the pg_catalog schema
58-
*during initdb. User-created relations in pg_catalog don't count as
59-
*system catalogs.
56+
*True iff the relation is either a system catalog or a toast table.
57+
*See IsCatalogRelation for the exact definition of a system catalog.
6058
*
61-
*NB: TOAST relations are considered system relations by this test
62-
*for compatibility with the old IsSystemRelationName function.
63-
*This is appropriate in many places but not all. Where it's not,
64-
*also check IsToastRelation or use IsCatalogRelation().
59+
*We treat toast tables of user relations as "system relations" for
60+
*protection purposes, e.g. you can't change their schemas without
61+
*special permissions. Therefore, most uses of this function are
62+
*checking whether allow_system_table_mods restrictions apply.
63+
*For other purposes, consider whether you shouldn't be using
64+
*IsCatalogRelation instead.
65+
*
66+
*This function does not perform any catalog accesses.
67+
*Some callers rely on that!
6568
*/
6669
bool
6770
IsSystemRelation(Relationrelation)
@@ -78,67 +81,74 @@ IsSystemRelation(Relation relation)
7881
bool
7982
IsSystemClass(Oidrelid,Form_pg_classreltuple)
8083
{
81-
returnIsToastClass(reltuple)||IsCatalogClass(relid,reltuple);
84+
/* IsCatalogRelationOid is a bit faster, so test that first */
85+
return (IsCatalogRelationOid(relid)||IsToastClass(reltuple));
8286
}
8387

8488
/*
8589
* IsCatalogRelation
86-
*True iff the relation is a system catalog, or the toast table for
87-
*a system catalog. By a system catalog, we mean one that created
88-
*in the pg_catalog schema during initdb. As with IsSystemRelation(),
89-
*user-created relations in pg_catalog don't count as system catalogs.
90+
*True iff the relation is a system catalog.
91+
*
92+
*By a system catalog, we mean one that is created during the bootstrap
93+
*phase of initdb. That includes not just the catalogs per se, but
94+
*also their indexes, and TOAST tables and indexes if any.
9095
*
91-
*Note that IsSystemRelation() returns true for ALL toast relations,
92-
*but this function returns true only for toast relations of system
93-
*catalogs.
96+
*This function does not perform any catalog accesses.
97+
*Some callers rely on that!
9498
*/
9599
bool
96100
IsCatalogRelation(Relationrelation)
97101
{
98-
returnIsCatalogClass(RelationGetRelid(relation),relation->rd_rel);
102+
returnIsCatalogRelationOid(RelationGetRelid(relation));
99103
}
100104

101105
/*
102-
* IsCatalogClass
103-
*True iff the relation is a system catalog relation.
106+
* IsCatalogRelationOid
107+
*True iff the relation identified by this OID is a system catalog.
108+
*
109+
*By a system catalog, we mean one that is created during the bootstrap
110+
*phase of initdb. That includes not just the catalogs per se, but
111+
*also their indexes, and TOAST tables and indexes if any.
104112
*
105-
* Check IsCatalogRelation() for details.
113+
*This function does not perform any catalog accesses.
114+
*Some callers rely on that!
106115
*/
107116
bool
108-
IsCatalogClass(Oidrelid,Form_pg_classreltuple)
117+
IsCatalogRelationOid(Oidrelid)
109118
{
110-
Oidrelnamespace=reltuple->relnamespace;
111-
112119
/*
113-
* Never consider relations outside pg_catalog/pg_toast to be catalog
114-
* relations.
115-
*/
116-
if (!IsSystemNamespace(relnamespace)&& !IsToastNamespace(relnamespace))
117-
return false;
118-
119-
/* ----
120-
* Check whether the oid was assigned during initdb, when creating the
121-
* initial template database. Minus the relations in information_schema
122-
* excluded above, these are integral part of the system.
123-
* We could instead check whether the relation is pinned in pg_depend, but
124-
* this is noticeably cheaper and doesn't require catalog access.
120+
* We consider a relation to be a system catalog if it has an OID that was
121+
* manually assigned or assigned by genbki.pl. This includes all the
122+
* defined catalogs, their indexes, and their TOAST tables and indexes.
125123
*
126-
* This test is safe since even an oid wraparound will preserve this
127-
* property (cf. GetNewObjectId()) and it has the advantage that it works
128-
* correctly even if a user decides to create a relation in the pg_catalog
129-
* namespace.
130-
* ----
124+
* This rule excludes the relations in information_schema, which are not
125+
* integral to the system and can be treated the same as user relations.
126+
* (Since it's valid to drop and recreate information_schema, any rule
127+
* that did not act this way would be wrong.)
128+
*
129+
* This test is reliable since an OID wraparound will skip this range of
130+
* OIDs; see GetNewObjectId().
131131
*/
132-
returnrelid<FirstNormalObjectId;
132+
return(relid<(Oid)FirstBootstrapObjectId);
133133
}
134134

135135
/*
136136
* IsToastRelation
137137
*True iff relation is a TOAST support relation (or index).
138+
*
139+
*Does not perform any catalog accesses.
138140
*/
139141
bool
140142
IsToastRelation(Relationrelation)
141143
{
144+
/*
145+
* What we actually check is whether the relation belongs to a pg_toast
146+
* namespace. This should be equivalent because of restrictions that are
147+
* enforced elsewhere against creating user relations in, or moving
148+
* relations into/out of, a pg_toast namespace. Notice also that this
149+
* will not say "true" for toast tables belonging to other sessions' temp
150+
* tables; we expect that other mechanisms will prevent access to those.
151+
*/
142152
returnIsToastNamespace(RelationGetNamespace(relation));
143153
}
144154

@@ -157,14 +167,16 @@ IsToastClass(Form_pg_class reltuple)
157167
}
158168

159169
/*
160-
*IsSystemNamespace
170+
*IsCatalogNamespace
161171
*True iff namespace is pg_catalog.
162172
*
173+
*Does not perform any catalog accesses.
174+
*
163175
* NOTE: the reason this isn't a macro is to avoid having to include
164176
* catalog/pg_namespace.h in a lot of places.
165177
*/
166178
bool
167-
IsSystemNamespace(OidnamespaceId)
179+
IsCatalogNamespace(OidnamespaceId)
168180
{
169181
returnnamespaceId==PG_CATALOG_NAMESPACE;
170182
}
@@ -173,9 +185,13 @@ IsSystemNamespace(Oid namespaceId)
173185
* IsToastNamespace
174186
*True iff namespace is pg_toast or my temporary-toast-table namespace.
175187
*
188+
*Does not perform any catalog accesses.
189+
*
176190
* Note: this will return false for temporary-toast-table namespaces belonging
177191
* to other backends. Those are treated the same as other backends' regular
178192
* temp table namespaces, and access is prevented where appropriate.
193+
* If you need to check for those, you may be able to use isAnyTempNamespace,
194+
* but beware that that does involve a catalog access.
179195
*/
180196
bool
181197
IsToastNamespace(OidnamespaceId)

‎src/backend/catalog/heap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ heap_create(const char *relname,
324324
* user defined relation, not a system one.
325325
*/
326326
if (!allow_system_table_mods&&
327-
((IsSystemNamespace(relnamespace)&&relkind!=RELKIND_INDEX)||
327+
((IsCatalogNamespace(relnamespace)&&relkind!=RELKIND_INDEX)||
328328
IsToastNamespace(relnamespace))&&
329329
IsNormalProcessingMode())
330330
ereport(ERROR,

‎src/backend/catalog/pg_publication.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,16 @@ check_publication_add_relation(Relation targetrel)
9393
*
9494
* Note this also excludes all tables with relid < FirstNormalObjectId,
9595
* ie all tables created during initdb. This mainly affects the preinstalled
96-
* information_schema. (IsCatalogClass() only checks for these inside
97-
* pg_catalog and toast schemas.)
96+
* information_schema. (IsCatalogRelationOid() only excludes tables with
97+
* relid < FirstBootstrapObjectId, making that test rather redundant, but
98+
* really we should get rid of the FirstNormalObjectId test not
99+
* IsCatalogRelationOid.)
98100
*/
99101
staticbool
100102
is_publishable_class(Oidrelid,Form_pg_classreltuple)
101103
{
102104
returnreltuple->relkind==RELKIND_RELATION&&
103-
!IsCatalogClass(relid,reltuple)&&
105+
!IsCatalogRelationOid(relid)&&
104106
reltuple->relpersistence==RELPERSISTENCE_PERMANENT&&
105107
relid >=FirstNormalObjectId;
106108
}

‎src/backend/commands/indexcmds.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2590,15 +2590,11 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
25902590
continue;
25912591

25922592
/*
2593-
* Skip system tables that index_create() would reject to index
2594-
* concurrently. XXX We need the additional check for
2595-
* FirstNormalObjectId to skip information_schema tables, because
2596-
* IsCatalogClass() here does not cover information_schema, but the
2597-
* check in index_create() will error on the TOAST tables of
2598-
* information_schema tables.
2593+
* Skip system tables, since index_create() would reject indexing them
2594+
* concurrently (and it would likely fail if we tried).
25992595
*/
26002596
if (concurrent&&
2601-
(IsCatalogClass(relid,classtuple)||relid<FirstNormalObjectId))
2597+
IsCatalogRelationOid(relid))
26022598
{
26032599
if (!concurrent_warning)
26042600
ereport(WARNING,
@@ -2842,7 +2838,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
28422838
errmsg("concurrent reindex is not supported for shared relations")));
28432839

28442840
/* A system catalog cannot be reindexed concurrently */
2845-
if (IsSystemNamespace(get_rel_namespace(heapId)))
2841+
if (IsCatalogRelationOid(heapId))
28462842
ereport(ERROR,
28472843
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
28482844
errmsg("concurrent reindex is not supported for catalog relations")));

‎src/backend/commands/tablecmds.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12453,9 +12453,10 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt)
1245312453
* Also, explicitly avoid any shared tables, temp tables, or TOAST
1245412454
* (TOAST will be moved with the main table).
1245512455
*/
12456-
if (IsSystemNamespace(relForm->relnamespace) || relForm->relisshared ||
12456+
if (IsCatalogNamespace(relForm->relnamespace) ||
12457+
relForm->relisshared ||
1245712458
isAnyTempNamespace(relForm->relnamespace) ||
12458-
relForm->relnamespace == PG_TOAST_NAMESPACE)
12459+
IsToastNamespace(relForm->relnamespace))
1245912460
continue;
1246012461

1246112462
/* Only move the object type requested */

‎src/backend/utils/cache/relcache.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3316,8 +3316,8 @@ RelationBuildLocalRelation(const char *relname,
33163316
else
33173317
rel->rd_rel->relispopulated= true;
33183318

3319-
/* systemrelations and non-table objects don't have one */
3320-
if (!IsSystemNamespace(relnamespace)&&
3319+
/*set replica identity --systemcatalogs and non-tables don't have one */
3320+
if (!IsCatalogNamespace(relnamespace)&&
33213321
(relkind==RELKIND_RELATION||
33223322
relkind==RELKIND_MATVIEW||
33233323
relkind==RELKIND_PARTITIONED_TABLE))

‎src/include/catalog/catalog.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ extern bool IsCatalogRelation(Relation relation);
2424

2525
externboolIsSystemClass(Oidrelid,Form_pg_classreltuple);
2626
externboolIsToastClass(Form_pg_classreltuple);
27-
externboolIsCatalogClass(Oidrelid,Form_pg_classreltuple);
2827

29-
externboolIsSystemNamespace(OidnamespaceId);
28+
externboolIsCatalogRelationOid(Oidrelid);
29+
30+
externboolIsCatalogNamespace(OidnamespaceId);
3031
externboolIsToastNamespace(OidnamespaceId);
3132

3233
externboolIsReservedName(constchar*name);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp