Add regression coverage for REVOKE ADMIN OPTION
Hackers,
While working on a fix for dangling references to dropped roles in the pg_auth_members.grantor field, I happened to notice we entirely lack regression test coverage of the REVOKE ADMIN OPTION FOR form of the RevokeRoleStmt. I am unaware of any bugs in the current implementation, but future work on roles may benefit if we close the testing gap.
For consideration:
Attachments:
v1-0001-Add-test-for-REVOKE-ADMIN-OPTION.patchapplication/octet-stream; name=v1-0001-Add-test-for-REVOKE-ADMIN-OPTION.patch; x-unix-mode=0644Download+51-1
On 16 Nov 2021, at 00:58, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
While working on a fix for dangling references to dropped roles in the pg_auth_members.grantor field, I happened to notice we entirely lack regression test coverage of the REVOKE ADMIN OPTION FOR form of the RevokeRoleStmt. I am unaware of any bugs in the current implementation, but future work on roles may benefit if we close the testing gap.
LGTM. Reading this I realized that the GRANTED BY keyword for RevokeRoleStmt
isn't working as documented, it's not checking the role at all. I've sent a
diff for that with tests on the relevant thread, but I think it would be a good
to get this in too to boost coverage.
--
Daniel Gustafsson https://vmware.com/
On Nov 16, 2021, at 6:31 AM, Daniel Gustafsson <daniel@yesql.se> wrote:
On 16 Nov 2021, at 00:58, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
While working on a fix for dangling references to dropped roles in the pg_auth_members.grantor field, I happened to notice we entirely lack regression test coverage of the REVOKE ADMIN OPTION FOR form of the RevokeRoleStmt. I am unaware of any bugs in the current implementation, but future work on roles may benefit if we close the testing gap.
LGTM. Reading this I realized that the GRANTED BY keyword for RevokeRoleStmt
isn't working as documented, it's not checking the role at all. I've sent a
diff for that with tests on the relevant thread, but I think it would be a good
to get this in too to boost coverage.
Thanks for the review!
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 16 Nov 2021, at 15:32, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
Thanks for the review!
Pushed to master along with the bugfix it helped uncover, thanks!
--
Daniel Gustafsson https://vmware.com/