From: Tom Lane <tgl@sss.pgh.pa.us>
To: Stephen Frost <sfrost@snowman.net>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17062: Assert failed in RemoveRoleFromObjectPolicy() on DROP OWNED policy applied to duplicate role
Date: Fri, 18 Jun 2021 15:53:14 -0400
Message-ID: <1285021.1624045994@sss.pgh.pa.us> (raw)
In-Reply-To: <CAOuzzgpmUhyS4T3bG7+rtF7ZLPd0kp+-piCengitHdMN1-PHLg@mail.gmail.com>
References: <17062-11f471ae3199ca23@postgresql.org>
<1066940.1623964553@sss.pgh.pa.us>
<1067500.1623965562@sss.pgh.pa.us>
<1068096.1623966678@sss.pgh.pa.us>
<YMwUWpBb26FkNBqW@paquier.xyz>
<1233107.1624040308@sss.pgh.pa.us>
<CAOuzzgo_-Vz7OfavbBZuf5uDCh986Q2wDOjWUtvs7YkTvdKKEw@mail.gmail.com>
<1233946.1624041420@sss.pgh.pa.us>
<CAOuzzgpmUhyS4T3bG7+rtF7ZLPd0kp+-piCengitHdMN1-PHLg@mail.gmail.com>
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
Attachments:fix-RemoveRoleFromObjectPolicy-1.patch (text/x-diff, 3.2K) downloaddiff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.cindex 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;
}
/*
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: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, sfrost@snowman.net, michael@paquier.xyz, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17062: Assert failed in RemoveRoleFromObjectPolicy() on DROP OWNED policy applied to duplicate role
In-Reply-To: <1285021.1624045994@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox