public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Refactor recordExtObjInitPriv()
Date: Tue, 27 Dec 2022 09:56:10 +0100
Message-ID: <[email protected]> (raw)
Another aclchk.c refactoring patch, similar to [0] and [1].
Refactor recordExtObjInitPriv(): Instead of half a dozen of
mostly-duplicate conditional branches, write one common one that can
handle most catalogs. We already have all the information we need, such
as which system catalog corresponds to which catalog table and which
column is the ACL column.
[0]:
https://www.postgresql.org/message-id/[email protected]
[1]:
https://www.postgresql.org/message-id/flat/22c7e802-4e7d-8d87-8b71-cba95e6f4bcf%40enterprisedb.com
From 2982380987066d43e4d35db2157f98498a2a3185 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 27 Dec 2022 09:38:54 +0100
Subject: [PATCH] Refactor recordExtObjInitPriv()
Instead of half a dozen of mostly-duplicate conditional branches,
write one common one that can handle most catalogs. We already have
all the information we need, such as which system catalog corresponds
to which catalog table and which column is the ACL column.
---
src/backend/catalog/aclchk.c | 153 ++---------------------------------
1 file changed, 8 insertions(+), 145 deletions(-)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index cdfd637815..cf18401449 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -4241,9 +4241,6 @@ recordDependencyOnNewAcl(Oid classId, Oid objectId, int32 objsubId,
*
* For the object passed in, this will record its ACL (if any) and the ACLs of
* any sub-objects (eg: columns) into pg_init_privs.
- *
- * Any new kinds of objects which have ACLs associated with them and can be
- * added to an extension should be added to the if-else tree below.
*/
void
recordExtObjInitPriv(Oid objoid, Oid classoid)
@@ -4336,74 +4333,6 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
ReleaseSysCache(tuple);
}
- /* pg_foreign_data_wrapper */
- else if (classoid == ForeignDataWrapperRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(FOREIGNDATAWRAPPEROID,
- ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for foreign data wrapper %u",
- objoid);
-
- aclDatum = SysCacheGetAttr(FOREIGNDATAWRAPPEROID, tuple,
- Anum_pg_foreign_data_wrapper_fdwacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_foreign_server */
- else if (classoid == ForeignServerRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(FOREIGNSERVEROID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for foreign server %u",
- objoid);
-
- aclDatum = SysCacheGetAttr(FOREIGNSERVEROID, tuple,
- Anum_pg_foreign_server_srvacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_language */
- else if (classoid == LanguageRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(LANGOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for language %u", objoid);
-
- aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
/* pg_largeobject_metadata */
else if (classoid == LargeObjectMetadataRelationId)
{
@@ -4446,60 +4375,21 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
systable_endscan(scan);
}
- /* pg_namespace */
- else if (classoid == NamespaceRelationId)
+ /* This will error on unsupported classoid. */
+ else if (get_object_attnum_acl(classoid))
{
Datum aclDatum;
bool isNull;
HeapTuple tuple;
- tuple = SearchSysCache1(NAMESPACEOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for schema %u", objoid);
-
- aclDatum = SysCacheGetAttr(NAMESPACEOID, tuple,
- Anum_pg_namespace_nspacl, &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_proc */
- else if (classoid == ProcedureRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for function %u", objoid);
-
- aclDatum = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_proacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_type */
- else if (classoid == TypeRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(objoid));
+ tuple = SearchSysCache1(get_object_catcache_oid(classoid),
+ ObjectIdGetDatum(objoid));
if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for type %u", objoid);
+ elog(ERROR, "cache lookup failed for %s %u",
+ get_object_class_descr(classoid), objoid);
- aclDatum = SysCacheGetAttr(TYPEOID, tuple, Anum_pg_type_typacl,
+ aclDatum = SysCacheGetAttr(get_object_catcache_oid(classoid), tuple,
+ get_object_attnum_acl(classoid),
&isNull);
/* Add the record, if any, for the top-level object */
@@ -4509,33 +4399,6 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
ReleaseSysCache(tuple);
}
- else if (classoid == AccessMethodRelationId ||
- classoid == AggregateRelationId ||
- classoid == CastRelationId ||
- classoid == CollationRelationId ||
- classoid == ConversionRelationId ||
- classoid == EventTriggerRelationId ||
- classoid == OperatorRelationId ||
- classoid == OperatorClassRelationId ||
- classoid == OperatorFamilyRelationId ||
- classoid == TSConfigRelationId ||
- classoid == TSDictionaryRelationId ||
- classoid == TSParserRelationId ||
- classoid == TSTemplateRelationId ||
- classoid == TransformRelationId
- )
- {
- /* no ACL for these object types, so do nothing. */
- }
-
- /*
- * complain if we are given a class OID for a class that extensions don't
- * support or that we don't recognize.
- */
- else
- {
- elog(ERROR, "unrecognized or unsupported class OID: %u", classoid);
- }
}
/*
--
2.39.0
Attachments:
[text/plain] 0001-Refactor-recordExtObjInitPriv.patch (6.3K, ../[email protected]/2-0001-Refactor-recordExtObjInitPriv.patch)
download | inline diff:
From 2982380987066d43e4d35db2157f98498a2a3185 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 27 Dec 2022 09:38:54 +0100
Subject: [PATCH] Refactor recordExtObjInitPriv()
Instead of half a dozen of mostly-duplicate conditional branches,
write one common one that can handle most catalogs. We already have
all the information we need, such as which system catalog corresponds
to which catalog table and which column is the ACL column.
---
src/backend/catalog/aclchk.c | 153 ++---------------------------------
1 file changed, 8 insertions(+), 145 deletions(-)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index cdfd637815..cf18401449 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -4241,9 +4241,6 @@ recordDependencyOnNewAcl(Oid classId, Oid objectId, int32 objsubId,
*
* For the object passed in, this will record its ACL (if any) and the ACLs of
* any sub-objects (eg: columns) into pg_init_privs.
- *
- * Any new kinds of objects which have ACLs associated with them and can be
- * added to an extension should be added to the if-else tree below.
*/
void
recordExtObjInitPriv(Oid objoid, Oid classoid)
@@ -4336,74 +4333,6 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
ReleaseSysCache(tuple);
}
- /* pg_foreign_data_wrapper */
- else if (classoid == ForeignDataWrapperRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(FOREIGNDATAWRAPPEROID,
- ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for foreign data wrapper %u",
- objoid);
-
- aclDatum = SysCacheGetAttr(FOREIGNDATAWRAPPEROID, tuple,
- Anum_pg_foreign_data_wrapper_fdwacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_foreign_server */
- else if (classoid == ForeignServerRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(FOREIGNSERVEROID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for foreign server %u",
- objoid);
-
- aclDatum = SysCacheGetAttr(FOREIGNSERVEROID, tuple,
- Anum_pg_foreign_server_srvacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_language */
- else if (classoid == LanguageRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(LANGOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for language %u", objoid);
-
- aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
/* pg_largeobject_metadata */
else if (classoid == LargeObjectMetadataRelationId)
{
@@ -4446,60 +4375,21 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
systable_endscan(scan);
}
- /* pg_namespace */
- else if (classoid == NamespaceRelationId)
+ /* This will error on unsupported classoid. */
+ else if (get_object_attnum_acl(classoid))
{
Datum aclDatum;
bool isNull;
HeapTuple tuple;
- tuple = SearchSysCache1(NAMESPACEOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for schema %u", objoid);
-
- aclDatum = SysCacheGetAttr(NAMESPACEOID, tuple,
- Anum_pg_namespace_nspacl, &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_proc */
- else if (classoid == ProcedureRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(objoid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for function %u", objoid);
-
- aclDatum = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_proacl,
- &isNull);
-
- /* Add the record, if any, for the top-level object */
- if (!isNull)
- recordExtensionInitPrivWorker(objoid, classoid, 0,
- DatumGetAclP(aclDatum));
-
- ReleaseSysCache(tuple);
- }
- /* pg_type */
- else if (classoid == TypeRelationId)
- {
- Datum aclDatum;
- bool isNull;
- HeapTuple tuple;
-
- tuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(objoid));
+ tuple = SearchSysCache1(get_object_catcache_oid(classoid),
+ ObjectIdGetDatum(objoid));
if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for type %u", objoid);
+ elog(ERROR, "cache lookup failed for %s %u",
+ get_object_class_descr(classoid), objoid);
- aclDatum = SysCacheGetAttr(TYPEOID, tuple, Anum_pg_type_typacl,
+ aclDatum = SysCacheGetAttr(get_object_catcache_oid(classoid), tuple,
+ get_object_attnum_acl(classoid),
&isNull);
/* Add the record, if any, for the top-level object */
@@ -4509,33 +4399,6 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
ReleaseSysCache(tuple);
}
- else if (classoid == AccessMethodRelationId ||
- classoid == AggregateRelationId ||
- classoid == CastRelationId ||
- classoid == CollationRelationId ||
- classoid == ConversionRelationId ||
- classoid == EventTriggerRelationId ||
- classoid == OperatorRelationId ||
- classoid == OperatorClassRelationId ||
- classoid == OperatorFamilyRelationId ||
- classoid == TSConfigRelationId ||
- classoid == TSDictionaryRelationId ||
- classoid == TSParserRelationId ||
- classoid == TSTemplateRelationId ||
- classoid == TransformRelationId
- )
- {
- /* no ACL for these object types, so do nothing. */
- }
-
- /*
- * complain if we are given a class OID for a class that extensions don't
- * support or that we don't recognize.
- */
- else
- {
- elog(ERROR, "unrecognized or unsupported class OID: %u", classoid);
- }
}
/*
--
2.39.0
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Refactor recordExtObjInitPriv()
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox