* Andres Freund (andres@anarazel.de) wrote:
> I find that a somewhat ugly coding pattern, but since the rest of the
> function is written that way...
Agreed, but not going to change it at this point.
Would love feedback on the attached. I included the variable renames
discussed previously with Noah as they're quite minor changes.
Had no trouble cherry-picking this back to 9.5.
> > I'll do that and add regression tests for it and any others which don't
> > get tested. My thinking on the test is to independently change each
> > value and then poll for all role attributes set and verify that the only
> > change made was the change expected.
>
> Do that if you like, but what I really think we should have is a test
> that tries to bypass rls and fails, then the user gets changes and it
> succeeds, and then it gets disabled and fails again. This really seems
> test-worthy behaviour to me.
I'll look at doing this also in the rowsecurity regression suite, but I
really like having this coverage of CREATE/ALTER ROLE too, plus testing
the role dump/restore paths in pg_dumpall which I don't think were being
covered at all previously...
Thanks!
Stephen