Re: ALTER DEFAULT PRIVILEGES is buggy, and so is its testing
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2018-11-09T17:45:40Z
Lists: pgsql-hackers
Attachments
- fix-default-privileges-dependencies-1.patch (text/x-diff) patch
I wrote: > So it seems like testing ALTER DEFAULT PRIVILEGES in a script that > runs in parallel with anything else is a damfool idea. But there > is also a live bug of failure to correctly record privileges granted > this way. Without that bug, we'd have had our noses rubbed in the > parallel race conditions long ago. It turns out the bug only applies to schemas and types; the other callers of get_user_default_acl() already had ad-hoc code to deal with the problem. I made that a bit less ad-hoc by creating a subroutine to do the work and documenting the need to call it. To fix the tests' race condition, it seems to be sufficient to wrap a transaction around the section of privileges.sql that grants and then revokes global default privileges. The other uses of ALTER DEFAULT PRIVILEGES in the tests are targeted narrowly enough to ensure that concurrent tests shouldn't pick them up. In the attached proposed patch for HEAD, since I had to adjust the API of GenerateTypeDependencies anyway, I changed it to pass the new pg_type row and get most of the old arguments out of that, as the argument list was getting pretty unwieldy. I suppose we could make a back-branch patch that doesn't change that API and instead makes the callers responsible for recording ACL dependencies, but ugh. Does anyone think it's likely that third-party code is calling GenerateTypeDependencies? regards, tom lane
Commits
-
Fix missing role dependencies for some schema and type ACLs.
- fa2952d8eb04 12.0 landed
- d431dff1af8c 9.6.12 landed
- 47088c599cc6 9.5.16 landed
- 2d83863ea273 10.7 landed
- 277602dfee51 9.4.21 landed
- 1b55acb2cf48 11.2 landed