Re: Add regression coverage for REVOKE ADMIN OPTION
> On 16 Nov 2021, at 15:32, Mark Dilger wrote: > Thanks for the review! Pushed to master along with the bugfix it helped uncover, thanks! -- Daniel Gustafsson https://vmware.com/
Re: Add regression coverage for REVOKE ADMIN OPTION
> On Nov 16, 2021, at 6:31 AM, Daniel Gustafsson wrote: > >> On 16 Nov 2021, at 00:58, Mark Dilger 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
Re: Add regression coverage for REVOKE ADMIN OPTION
> On 16 Nov 2021, at 00:58, Mark Dilger 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/
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: v1-0001-Add-test-for-REVOKE-ADMIN-OPTION.patch Description: Binary data — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
