Improve reporting of permission errors for array types

Because permissions are assigned to element types, not array types,
complaining about permission denied on an array type would be
misleading to users.  So adjust the reporting to refer to the element
type instead.

In order not to duplicate the required logic in two dozen places,
refactor the permission denied reporting for types a bit.

pointed out by Yeb Havinga during the review of the type privilege
feature

diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 1f40b7c6604d68eaf2f6ed9ab1a120ce560f9706..aa1ce800d40405e5759c9ab48800421f0229d564 100644 (file)
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -573,8 +573,7 @@ BuildDescForRelation(List *schema)
 
        aclresult = pg_type_aclcheck(atttypid, GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(atttypid));
+           aclcheck_error_type(aclresult, atttypid);
 
        attcollation = GetColumnDefCollation(NULL, entry, atttypid);
        attdim = list_length(entry->typeName->arrayBounds);

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 45cd0808ce80c609373de189bb330659c54b9895..56c40b14e002ea7ccd892c8f086a29efb5f55805 100644 (file)
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3389,6 +3389,19 @@ aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind,
 }
 
 
+/*
+ * Special common handling for types: use element type instead of array type,
+ * and format nicely
+ */
+void
+aclcheck_error_type(AclResult aclerr, Oid typeOid)
+{
+   Oid element_type = get_element_type(typeOid);
+
+   aclcheck_error(aclerr, ACL_KIND_TYPE, format_type_be(element_type ? element_type : typeOid));
+}
+
+
 /* Check if given user has rolcatupdate privilege according to pg_authid */
 static bool
 has_rolcatupdate(Oid roleid)

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 5a06fcbf41d5184adb538768b8f24747c2b687e2..19bde9f4761f3c51668737824c41dcd823c3350d 100644 (file)
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -937,8 +937,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
        case OBJECT_DOMAIN:
        case OBJECT_ATTRIBUTE:
            if (!pg_type_ownercheck(address.objectId, roleid))
-               aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                              format_type_be(address.objectId));
+               aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
            break;
        case OBJECT_AGGREGATE:
        case OBJECT_FUNCTION:

diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 0b393cf6ceeaa668c65b00bdb50533b973a91b74..403c6f03a630b2079144bd70433cf7f4b2b1d69b 100644 (file)
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -208,19 +208,16 @@ AggregateCreate(const char *aggName,
    {
        aclresult = pg_type_aclcheck(aggArgTypes[i], GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(aggArgTypes[i]));
+           aclcheck_error_type(aclresult, aggArgTypes[i]);
    }
 
    aclresult = pg_type_aclcheck(aggTransType, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(aggTransType));
+       aclcheck_error_type(aclresult, aggTransType);
 
    aclresult = pg_type_aclcheck(finaltype, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(finaltype));
+       aclcheck_error_type(aclresult, finaltype);
 
 
    /*

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 13e30f4a556b993de247498f5a5e412acb68b77c..9ba6dd8fcf19ce500ca80163aed637aa3e11d983 100644 (file)
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -154,8 +154,7 @@ compute_return_type(TypeName *returnType, Oid languageOid,
 
    aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(rettype));
+       aclcheck_error_type(aclresult, rettype);
 
    *prorettype_p = rettype;
    *returnsSet_p = returnType->setof;
@@ -247,8 +246,7 @@ examine_parameter_list(List *parameters, Oid languageOid,
 
        aclresult = pg_type_aclcheck(toid, GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(toid));
+           aclcheck_error_type(aclresult, toid);
 
        if (t->setof)
            ereport(ERROR,
@@ -1510,13 +1508,11 @@ CreateCast(CreateCastStmt *stmt)
 
    aclresult = pg_type_aclcheck(sourcetypeid, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(sourcetypeid));
+       aclcheck_error_type(aclresult, sourcetypeid);
 
    aclresult = pg_type_aclcheck(targettypeid, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(targettypeid));
+       aclcheck_error_type(aclresult, targettypeid);
 
    /* Domains are allowed for historical reasons, but we warn */
    if (sourcetyptype == TYPTYPE_DOMAIN)

diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index 460b1d9ae288a73937106216f55dd32675a67668..f5642447f12280edacbdd7d3a0a5d944b2b8d1fb 100644 (file)
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -414,8 +414,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
    /* XXX this is unnecessary given the superuser check above */
    /* Check we have ownership of the datatype */
    if (!pg_type_ownercheck(typeoid, GetUserId()))
-       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                      format_type_be(typeoid));
+       aclcheck_error_type(ACLCHECK_NOT_OWNER, typeoid);
 #endif
 
    /*
@@ -565,8 +564,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
                /* XXX this is unnecessary given the superuser check above */
                /* Check we have ownership of the datatype */
                if (!pg_type_ownercheck(storageoid, GetUserId()))
-                   aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                                  format_type_be(storageoid));
+                   aclcheck_error_type(ACLCHECK_NOT_OWNER, storageoid);
 #endif
                break;
            default:

diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index afae5e406e87628c6c9df693e94ae784261e0594..410c708975f487fbfc18389b950deafbbe8287c7 100644 (file)
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -181,16 +181,14 @@ DefineOperator(List *names, List *parameters)
    {
        aclresult = pg_type_aclcheck(typeId1, GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(typeId1));
+           aclcheck_error_type(aclresult, typeId1);
    }
 
    if (typeName2)
    {
        aclresult = pg_type_aclcheck(typeId2, GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(typeId2));
+           aclcheck_error_type(aclresult, typeId2);
    }
 
    /*
@@ -227,8 +225,7 @@ DefineOperator(List *names, List *parameters)
    rettype = get_func_rettype(functionOid);
    aclresult = pg_type_aclcheck(rettype, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(rettype));
+       aclcheck_error_type(aclresult, rettype);
 
    /*
     * Look up restriction estimator if specified

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5c69cfb85a2e1b5b961a252780b2024f33c6a1dd..70753e33e430237a66991bac0740b8761c1b4e4e 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -526,8 +526,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
 
        aclresult = pg_type_aclcheck(ofTypeId, GetUserId(), ACL_USAGE);
        if (aclresult != ACLCHECK_OK)
-           aclcheck_error(aclresult, ACL_KIND_TYPE,
-                          format_type_be(ofTypeId));
+           aclcheck_error_type(aclresult, ofTypeId);
    }
    else
        ofTypeId = InvalidOid;
@@ -4500,8 +4499,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 
    aclresult = pg_type_aclcheck(typeOid, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(typeOid));
+       aclcheck_error_type(aclresult, typeOid);
 
    collOid = GetColumnDefCollation(NULL, colDef, typeOid);
 
@@ -7248,8 +7246,7 @@ ATPrepAlterColumnType(List **wqueue,
 
    aclresult = pg_type_aclcheck(targettype, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(targettype));
+       aclcheck_error_type(aclresult, targettype);
 
    /* And the collation */
    targetcollid = GetColumnDefCollation(NULL, def, targettype);

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index fdb5bdbc11ba65da7044959658965b8955215ea4..30850b2b22e20d9d916dddc92ee9db9584416f39 100644 (file)
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -758,8 +758,7 @@ DefineDomain(CreateDomainStmt *stmt)
 
    aclresult = pg_type_aclcheck(basetypeoid, GetUserId(), ACL_USAGE);
    if (aclresult != ACLCHECK_OK)
-       aclcheck_error(aclresult, ACL_KIND_TYPE,
-                      format_type_be(basetypeoid));
+       aclcheck_error_type(aclresult, basetypeoid);
 
    /*
     * Identify the collation if any
@@ -1208,8 +1207,7 @@ checkEnumOwner(HeapTuple tup)
 
    /* Permission check: must own type */
    if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
-       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                      format_type_be(HeapTupleGetOid(tup)));
+       aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
 }
 
 
@@ -2809,8 +2807,7 @@ checkDomainOwner(HeapTuple tup)
 
    /* Permission check: must own type */
    if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
-       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                      format_type_be(HeapTupleGetOid(tup)));
+       aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
 }
 
 /*
@@ -3116,8 +3113,7 @@ RenameType(RenameStmt *stmt)
 
    /* check permissions on type */
    if (!pg_type_ownercheck(typeOid, GetUserId()))
-       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                      format_type_be(typeOid));
+       aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
 
    /* ALTER DOMAIN used on a non-domain? */
    if (stmt->renameType == OBJECT_DOMAIN && typTup->typtype != TYPTYPE_DOMAIN)
@@ -3238,8 +3234,7 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
        {
            /* Otherwise, must be owner of the existing object */
            if (!pg_type_ownercheck(HeapTupleGetOid(tup), GetUserId()))
-               aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                              format_type_be(HeapTupleGetOid(tup)));
+               aclcheck_error_type(ACLCHECK_NOT_OWNER, HeapTupleGetOid(tup));
 
            /* Must be able to become new owner */
            check_is_member_of_role(GetUserId(), newOwnerId);
@@ -3367,8 +3362,7 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid)
 
    /* check permissions on type */
    if (!pg_type_ownercheck(typeOid, GetUserId()))
-       aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE,
-                      format_type_be(typeOid));
+       aclcheck_error_type(ACLCHECK_NOT_OWNER, typeOid);
 
    /* don't allow direct alteration of array types */
    elemOid = get_element_type(typeOid);

diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 6de39b21cf4f01972a7c5b965af65fd29a3ee5cd..2d1cccbf66e166845a5926119504b37d245cc9f0 100644 (file)
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -302,6 +302,8 @@ extern void aclcheck_error(AclResult aclerr, AclObjectKind objectkind,
 extern void aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind,
                   const char *objectname, const char *colname);
 
+extern void aclcheck_error_type(AclResult aclerr, Oid typeOid);
+
 /* ownercheck routines just return true (owner) or false (not) */
 extern bool pg_class_ownercheck(Oid class_oid, Oid roleid);
 extern bool pg_type_ownercheck(Oid type_oid, Oid roleid);

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index dfaf3a7a7e01146f711edad112e47c035219bae7..f99ebde33cc61c863220ddca1d0071809a4ab00b 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -547,7 +547,7 @@ ERROR:  permission denied for type testdomain1
 CREATE TABLE test6a OF testtype1;
 ERROR:  permission denied for type testtype1
 CREATE TABLE test10a (a int[], b testtype1[]);
-ERROR:  permission denied for type testtype1[]
+ERROR:  permission denied for type testtype1
 CREATE TABLE test9a (a int, b int);
 ALTER TABLE test9a ADD COLUMN c testdomain1;
 ERROR:  permission denied for type testdomain1