Allow makeaclitem() to accept multiple privilege names.

Interpret its privileges argument as a comma-separated list of
privilege names, as in has_table_privilege and other functions.
This is actually net less code, since the support routine to
parse that already exists, and we can drop convert_priv_string()
which had no other use-case.

Robins Tharakan

Discussion: https://postgr.es/m/e5a05dc54ba64408b3dd260171c1abaf@EX13D05UWC001.ant.amazon.com

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7b652460a18fa807fe56937b0547f929a71ccb98..1705b0b4eac9b39c98d796b43224b82bee3dba87 100644 (file)
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -24236,7 +24236,8 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute');
         If the grantee is the pseudo-role PUBLIC, it is represented by zero in
         the <parameter>grantee</parameter> column.  Each granted privilege is
         represented as <literal>SELECT</literal>, <literal>INSERT</literal>,
-        etc.  Note that each privilege is broken out as a separate row, so
+        etc (see <xref linkend="privilege-abbrevs-table"/> for a full list).
+        Note that each privilege is broken out as a separate row, so
         only one keyword appears in the <parameter>privilege_type</parameter>
         column.
        </para></entry>
@@ -24256,6 +24257,12 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute');
        </para>
        <para>
         Constructs an <type>aclitem</type> with the given properties.
+        <parameter>privileges</parameter> is a comma-separated list of
+        privilege names such as <literal>SELECT</literal>,
+        <literal>INSERT</literal>, etc, all of which are set in the
+        result.  (Case of the privilege string is not significant, and
+        extra whitespace is allowed between but not within privilege
+        names.)
        </para></entry>
       </row>
      </tbody>

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 772c04155c32de64401f34c309a04933ac33e842..b7fd3bcf057c69d3c6a58707dfbc6564c48d9bb6 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -86,7 +86,6 @@ static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
 static Acl *recursive_revoke(Acl *acl, Oid grantee, AclMode revoke_privs,
                             Oid ownerId, DropBehavior behavior);
 
-static AclMode convert_priv_string(text *priv_type_text);
 static AclMode convert_any_priv_string(text *priv_type_text,
                                       const priv_map *privileges);
 
@@ -1573,8 +1572,27 @@ makeaclitem(PG_FUNCTION_ARGS)
    bool        goption = PG_GETARG_BOOL(3);
    AclItem    *result;
    AclMode     priv;
+   static const priv_map any_priv_map[] = {
+       {"SELECT", ACL_SELECT},
+       {"INSERT", ACL_INSERT},
+       {"UPDATE", ACL_UPDATE},
+       {"DELETE", ACL_DELETE},
+       {"TRUNCATE", ACL_TRUNCATE},
+       {"REFERENCES", ACL_REFERENCES},
+       {"TRIGGER", ACL_TRIGGER},
+       {"EXECUTE", ACL_EXECUTE},
+       {"USAGE", ACL_USAGE},
+       {"CREATE", ACL_CREATE},
+       {"TEMP", ACL_CREATE_TEMP},
+       {"TEMPORARY", ACL_CREATE_TEMP},
+       {"CONNECT", ACL_CONNECT},
+       {"SET", ACL_SET},
+       {"ALTER SYSTEM", ACL_ALTER_SYSTEM},
+       {"RULE", 0},            /* ignore old RULE privileges */
+       {NULL, 0}
+   };
 
-   priv = convert_priv_string(privtext);
+   priv = convert_any_priv_string(privtext, any_priv_map);
 
    result = (AclItem *) palloc(sizeof(AclItem));
 
@@ -1587,50 +1605,6 @@ makeaclitem(PG_FUNCTION_ARGS)
    PG_RETURN_ACLITEM_P(result);
 }
 
-static AclMode
-convert_priv_string(text *priv_type_text)
-{
-   char       *priv_type = text_to_cstring(priv_type_text);
-
-   if (pg_strcasecmp(priv_type, "SELECT") == 0)
-       return ACL_SELECT;
-   if (pg_strcasecmp(priv_type, "INSERT") == 0)
-       return ACL_INSERT;
-   if (pg_strcasecmp(priv_type, "UPDATE") == 0)
-       return ACL_UPDATE;
-   if (pg_strcasecmp(priv_type, "DELETE") == 0)
-       return ACL_DELETE;
-   if (pg_strcasecmp(priv_type, "TRUNCATE") == 0)
-       return ACL_TRUNCATE;
-   if (pg_strcasecmp(priv_type, "REFERENCES") == 0)
-       return ACL_REFERENCES;
-   if (pg_strcasecmp(priv_type, "TRIGGER") == 0)
-       return ACL_TRIGGER;
-   if (pg_strcasecmp(priv_type, "EXECUTE") == 0)
-       return ACL_EXECUTE;
-   if (pg_strcasecmp(priv_type, "USAGE") == 0)
-       return ACL_USAGE;
-   if (pg_strcasecmp(priv_type, "CREATE") == 0)
-       return ACL_CREATE;
-   if (pg_strcasecmp(priv_type, "TEMP") == 0)
-       return ACL_CREATE_TEMP;
-   if (pg_strcasecmp(priv_type, "TEMPORARY") == 0)
-       return ACL_CREATE_TEMP;
-   if (pg_strcasecmp(priv_type, "CONNECT") == 0)
-       return ACL_CONNECT;
-   if (pg_strcasecmp(priv_type, "SET") == 0)
-       return ACL_SET;
-   if (pg_strcasecmp(priv_type, "ALTER SYSTEM") == 0)
-       return ACL_ALTER_SYSTEM;
-   if (pg_strcasecmp(priv_type, "RULE") == 0)
-       return 0;               /* ignore old RULE privileges */
-
-   ereport(ERROR,
-           (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-            errmsg("unrecognized privilege type: \"%s\"", priv_type)));
-   return ACL_NO_RIGHTS;       /* keep compiler quiet */
-}
-
 
 /*
  * convert_any_priv_string: recognize privilege strings for has_foo_privilege

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 03df567d50f6699f32ff96ab5409609b3f288000..e10dd6f9ae5a171e18e2b801795901dbe55c991f 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -2099,6 +2099,24 @@ SELECT has_table_privilege('regress_priv_user1', 'testns.acltest1', 'INSERT'); -
 ALTER DEFAULT PRIVILEGES FOR ROLE regress_priv_user1 REVOKE EXECUTE ON FUNCTIONS FROM public;
 ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error
 ERROR:  cannot use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS
+-- Test makeaclitem()
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT', TRUE);  -- single privilege
+               makeaclitem                
+------------------------------------------
+ regress_priv_user1=r*/regress_priv_user2
+(1 row)
+
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT, INSERT,  UPDATE , DELETE  ', FALSE);  -- multiple privileges
+                makeaclitem                 
+--------------------------------------------
+ regress_priv_user1=arwd/regress_priv_user2
+(1 row)
+
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT, fake_privilege', FALSE);  -- error
+ERROR:  unrecognized privilege type: "fake_privilege"
 --
 -- Testing blanket default grants is very hazardous since it might change
 -- the privileges attached to objects created by concurrent regression tests.

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 2a6ba38e523729444b31ae648e29af21065e5a30..6d1fd3391a208717aecd77292a4d88251e684e95 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1339,6 +1339,14 @@ ALTER DEFAULT PRIVILEGES FOR ROLE regress_priv_user1 REVOKE EXECUTE ON FUNCTIONS
 
 ALTER DEFAULT PRIVILEGES IN SCHEMA testns GRANT USAGE ON SCHEMAS TO regress_priv_user2; -- error
 
+-- Test makeaclitem()
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT', TRUE);  -- single privilege
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT, INSERT,  UPDATE , DELETE  ', FALSE);  -- multiple privileges
+SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
+   'SELECT, fake_privilege', FALSE);  -- error
+
 --
 -- Testing blanket default grants is very hazardous since it might change
 -- the privileges attached to objects created by concurrent regression tests.