"Bossart, Nathan" <bossartn@amazon.com> writes:
> The problem appears to be that pg_dump is comparing the entries in
> pg_default_acl to the default ACL (i.e., acldefault()). This is fine
> for "global" entries (i.e., entries with no schema specified), but it
> doesn't work for "non-global" entries (i.e., entries with a schema
> specified). This is because the default for a non-global entry is
> actually an empty ACL.
Good point.
> I've attached a quick hack that seems to fix this by adjusting the
> pg_dump query to use NULL instead of acldefault() for non-global
> entries. I'm posting this early in order to gather thoughts on the
> approach and to make sure I'm not missing something obvious.
I find this impossible to comment on as to correctness, because the patch
is nigh unreadable. "case_stmt" is a pretty darn opaque variable name,
and the lack of comments doesn't help, and I don't really think that you
chose good semantics for it anyway. Probably it would be better for the
new argument to be along the lines of "bool is_default_acl", and allow
buildACLQueries to know what it should put in when that's true.
I'm kind of allergic to this SQL coding style, too. It expects the
backend to expend many thousands of cycles parsing and then optimizing
away a useless CASE, to save a couple of lines of code and a few cycles
on the client side. Nor is doing the query this way even particularly
readable on the client side.
Lastly, there probably should be a test case or two.
regards, tom lane
