Re: pg_dump handling of ALTER DEFAULT PRIVILEGES IN SCHEMA

"Bossart, Nathan" <bossartn(at)amazon(dot)com> writes:
> On 9/5/21, 10:08 AM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 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.

> Is there a specific style you have in mind for this change, or is your
> point that the CASE statement should only be reserved for when
> is_default_acl is true?

I'd be inclined to split this logic out into a separate if-statement,
along the lines of

... build some of the query ...

if (is_default_acl)
{
/* The reference ACL is empty for schema-local default ACLs */
appendPQExpBuffer(..., "CASE WHEN ... THEN pg_catalog.acldefault(%s,%s) ELSE NULL END", ...);
}
else
{
/* For other cases, the reference is always acldefault() */
appendPQExpBuffer(..., "pg_catalog.acldefault(%s,%s)", ...);
}

... build rest of the query ...

I think this is more readable than one giant printf, and not incidentally
it allows room for some helpful comments.

(I kind of wonder if we shouldn't try to refactor buildACLQueries to
reduce code duplication and add comments while we're at it. The existing
code seems pretty awful from a readability standpoint already. I don't
have any clear idea about what to do instead, though.)

regards, tom lane