Re: BUG #17062: Assert failed in RemoveRoleFromObjectPolicy() on DROP OWNED policy applied to duplicate role - Mailing list pgsql-bugs

Here's a stop-the-bleeding patch that just gets rid of the assertion
failure and the tuple-updated-by-self problem.  I think this is
reasonable to slip in for beta2, although the other issues we
mentioned seem to require more thought.

(For ease of review, I've not re-pgindented.)

            regards, tom lane

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 5d469309ce..abb6c66316 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -18,6 +18,7 @@
 #include "access/relation.h"
 #include "access/sysattr.h"
 #include "access/table.h"
+#include "access/xact.h"
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
@@ -425,7 +426,6 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
     Oid            relid;
     Relation    rel;
     ArrayType  *policy_roles;
-    int            num_roles;
     Datum        roles_datum;
     bool        attr_isnull;
     bool        noperm = true;
@@ -481,11 +481,6 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)

     policy_roles = DatumGetArrayTypePCopy(roles_datum);

-    /* We should be removing exactly one entry from the roles array */
-    num_roles = ARR_DIMS(policy_roles)[0] - 1;
-
-    Assert(num_roles >= 0);
-
     /* Must own relation. */
     if (pg_class_ownercheck(relid, GetUserId()))
         noperm = false;            /* user is allowed to modify this policy */
@@ -497,15 +492,13 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
                         NameStr(((Form_pg_policy) GETSTRUCT(tuple))->polname),
                         RelationGetRelationName(rel))));

-    /*
-     * If multiple roles exist on this policy, then remove the one we were
-     * asked to and leave the rest.
-     */
-    if (!noperm && num_roles > 0)
+    /* Skip the rest if we lack permissions. */
+    if (!noperm)
     {
         int            i,
                     j;
         Oid           *roles = (Oid *) ARR_DATA_PTR(policy_roles);
+        int            num_roles = ARR_DIMS(policy_roles)[0];
         Datum       *role_oids;
         char       *qual_value;
         Node       *qual_expr;
@@ -582,16 +575,22 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
         else
             with_check_qual = NULL;

-        /* Rebuild the roles array to then update the pg_policy tuple with */
+        /*
+         * Rebuild the roles array, without any mentions of the target role.
+         * Ordinarily there'd be exactly one, but we must cope with duplicate
+         * mentions, since CREATE/ALTER POLICY historically have allowed that.
+         */
         role_oids = (Datum *) palloc(num_roles * sizeof(Datum));
-        for (i = 0, j = 0; i < ARR_DIMS(policy_roles)[0]; i++)
-            /* Copy over all of the roles which are not the one being removed */
+        for (i = 0, j = 0; i < num_roles; i++)
+        {
             if (roles[i] != roleid)
                 role_oids[j++] = ObjectIdGetDatum(roles[i]);
+        }
+        num_roles = j;

-        /* We should have only removed the one role */
-        Assert(j == num_roles);
-
+        /* If no roles remain, we have to drop the policy instead. */
+        if (num_roles > 0)
+        {
         /* This is the array for the new tuple */
         role_ids = construct_array(role_oids, num_roles, OIDOID,
                                    sizeof(Oid), true, TYPALIGN_INT);
@@ -646,8 +645,15 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)

         heap_freetuple(new_tuple);

+        /* Make updates visible */
+        CommandCounterIncrement();
+
         /* Invalidate Relation Cache */
         CacheInvalidateRelcache(rel);
+
+        /* Report that policy should not be dropped */
+        noperm = true;
+    }
     }

     /* Clean up. */
@@ -657,7 +663,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)

     table_close(pg_policy_rel, RowExclusiveLock);

-    return (noperm || num_roles > 0);
+    return noperm;
 }

 /*