pgsql: Fix misbehavior of DROP OWNED BY with duplicate polroles entries
Fix misbehavior of DROP OWNED BY with duplicate polroles entries.
Ordinarily, a pg_policy.polroles array wouldn't list the same role
more than once; but CREATE POLICY does not prevent that. If we
perform DROP OWNED BY on a role that is listed more than once,
RemoveRoleFromObjectPolicy either suffered an assertion failure
or encountered a tuple-updated-by-self error. Rewrite it to cope
correctly with duplicate entries, and add a CommandCounterIncrement
call to prevent the other problem.
Per discussion, there's other cleanup that ought to happen here,
but this seems like the minimum essential fix.
Per bug #17062 from Alexander Lakhin. It's been broken all along,
so back-patch to all supported branches.
Discussion: /messages/by-id/17062-11f471ae3199ca23@postgresql.org
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/d21fca084356946664bfce19d66d2df2bb873cbd
Modified Files
--------------
src/backend/commands/policy.c | 50 +++++++++++++++++--------------
src/test/regress/expected/rowsecurity.out | 9 ++++++
src/test/regress/sql/rowsecurity.sql | 10 +++++++
3 files changed, 46 insertions(+), 23 deletions(-)
On Fri, Jun 18, 2021 at 10:00:33PM +0000, Tom Lane wrote:
Fix misbehavior of DROP OWNED BY with duplicate polroles entries.
Ordinarily, a pg_policy.polroles array wouldn't list the same role
more than once; but CREATE POLICY does not prevent that. If we
perform DROP OWNED BY on a role that is listed more than once,
RemoveRoleFromObjectPolicy either suffered an assertion failure
or encountered a tuple-updated-by-self error. Rewrite it to cope
correctly with duplicate entries, and add a CommandCounterIncrement
call to prevent the other problem.Per discussion, there's other cleanup that ought to happen here,
but this seems like the minimum essential fix.
A nit here.
+ /* If any roles remain, update the policy entry. */
+ if (num_roles > 0)
+ {
/* This is the array for the new tuple */
This indentation is incorrect.
--
MichaelMichael Paquier <michael@paquier.xyz> writes:
This indentation is incorrect.
Yeah, I intentionally didn't reindent the code that's now inside
the inner if-block. It would have made back-patching more painful
(because that chunk isn't identical across branches). And I think
we're going to be rewriting that whole function in a little bit,
so I see no need to be very fussy about how pretty the intermediate
state is. I just pushed that because I didn't want to ship beta2
with a known crasher bug.
regards, tom lane