Remove count_one_bits() in acl.c.

The only caller, select_best_grantor(), can instead use
pg_popcount64().  This isn't performance-critical code, but we
might as well use the centralized implementation.  While at it, add
some test coverage for this part of select_best_grantor().

Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/Z9GtL7Nm6hsYyJnF%40nathan

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 6a76550a5e2ddb1ab0b0bd3cb819fac3abc5bbe4..ba14713fef22f7aa4b8bec95ab5866826dc66cb5 100644 (file)
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -5432,24 +5432,6 @@ select_best_admin(Oid member, Oid role)
    return admin_role;
 }
 
-
-/* does what it says ... */
-static int
-count_one_bits(AclMode mask)
-{
-   int         nbits = 0;
-
-   /* this code relies on AclMode being an unsigned type */
-   while (mask)
-   {
-       if (mask & 1)
-           nbits++;
-       mask >>= 1;
-   }
-   return nbits;
-}
-
-
 /*
  * Select the effective grantor ID for a GRANT or REVOKE operation.
  *
@@ -5532,7 +5514,7 @@ select_best_grantor(Oid roleId, AclMode privileges,
         */
        if (otherprivs != ACL_NO_RIGHTS)
        {
-           int         nnewrights = count_one_bits(otherprivs);
+           int         nnewrights = pg_popcount64(otherprivs);
 
            if (nnewrights > nrights)
            {

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index a76256405fe27c73924fbd14315670f41df7f122..954f549555e2eeec4b4fa81f7d6fead2d8f88b0e 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -3293,3 +3293,30 @@ DROP SCHEMA reindex_test;
 DROP ROLE regress_no_maintain;
 DROP ROLE regress_maintain;
 DROP ROLE regress_maintain_all;
+-- grantor selection
+CREATE ROLE regress_grantor1;
+CREATE ROLE regress_grantor2 ROLE regress_grantor1;
+CREATE ROLE regress_grantor3;
+CREATE TABLE grantor_test1 ();
+CREATE TABLE grantor_test2 ();
+CREATE TABLE grantor_test3 ();
+GRANT SELECT ON grantor_test2 TO regress_grantor1 WITH GRANT OPTION;
+GRANT SELECT, UPDATE ON grantor_test3 TO regress_grantor2 WITH GRANT OPTION;
+SET ROLE regress_grantor1;
+GRANT SELECT, UPDATE ON grantor_test1 TO regress_grantor3;
+ERROR:  permission denied for table grantor_test1
+GRANT SELECT, UPDATE ON grantor_test2 TO regress_grantor3;
+WARNING:  not all privileges were granted for "grantor_test2"
+GRANT SELECT, UPDATE ON grantor_test3 TO regress_grantor3;
+RESET ROLE;
+SELECT * FROM information_schema.table_privileges t
+   WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+     grantor      |     grantee      | table_catalog | table_schema |  table_name   | privilege_type | is_grantable | with_hierarchy 
+------------------+------------------+---------------+--------------+---------------+----------------+--------------+----------------
+ regress_grantor1 | regress_grantor3 | regression    | public       | grantor_test2 | SELECT         | NO           | YES
+ regress_grantor2 | regress_grantor3 | regression    | public       | grantor_test3 | SELECT         | NO           | YES
+ regress_grantor2 | regress_grantor3 | regression    | public       | grantor_test3 | UPDATE         | NO           | NO
+(3 rows)
+
+DROP TABLE grantor_test1, grantor_test2, grantor_test3;
+DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3;

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index d195aaf137741ff26503741f3e752e37e06aa728..b81694c24f28a3d64da470794095eebeb1ac94a5 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -2042,3 +2042,25 @@ DROP SCHEMA reindex_test;
 DROP ROLE regress_no_maintain;
 DROP ROLE regress_maintain;
 DROP ROLE regress_maintain_all;
+
+-- grantor selection
+CREATE ROLE regress_grantor1;
+CREATE ROLE regress_grantor2 ROLE regress_grantor1;
+CREATE ROLE regress_grantor3;
+CREATE TABLE grantor_test1 ();
+CREATE TABLE grantor_test2 ();
+CREATE TABLE grantor_test3 ();
+GRANT SELECT ON grantor_test2 TO regress_grantor1 WITH GRANT OPTION;
+GRANT SELECT, UPDATE ON grantor_test3 TO regress_grantor2 WITH GRANT OPTION;
+
+SET ROLE regress_grantor1;
+GRANT SELECT, UPDATE ON grantor_test1 TO regress_grantor3;
+GRANT SELECT, UPDATE ON grantor_test2 TO regress_grantor3;
+GRANT SELECT, UPDATE ON grantor_test3 TO regress_grantor3;
+RESET ROLE;
+
+SELECT * FROM information_schema.table_privileges t
+   WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+
+DROP TABLE grantor_test1, grantor_test2, grantor_test3;
+DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3;