Re: PROPERTY GRAPH pg_dump ACL minimization

On Mon, Jul 06, 2026 at 12:40:05PM -0400, Robert Haas wrote:
> On Mon, Jun 29, 2026 at 10:33 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > pg_dump doesn't do its normal ACL minimization for the new PROPERTY GRAPH
> > feature. Patch attached. See log message for details.

> While reviewing this patch, I wondered whether quoteAclUserName()
> exactly matches what the server would do. It has this dire warning:
>
> /* This test had better match what putid() does */
>
> It doesn't any more, quite, because quoteAclUserName() uses this test:
>
> !isalnum((unsigned char) *src) && *src != '_'
>
> And putid uses this test:
>
> !is_safe_acl_char(*src, false)
>
> The difference between the two is that is_safe_acl_char(c) will return
> false for any bytes where IS_HIGHBIT_SET(c) returns true. So consider:
>
> CREATE ROLE álvaro;
> CREATE PROPERTY GRAPH herrera;
> ALTER PROPERTY GRAPH herrera OWNER TO álvaro;
> GRANT SELECT ON PROPERTY GRAPH herrera TO PUBLIC;
>
> In a UTF8 database, everything is fine. But with encoding = LATIN1 and
> lc_ctype = en_US.ISO8859-1, pg_class.relacl is display with quotes
> around álvaro, and the string pg_dump synthesizes lacks them. This
> doesn't seem to cause a functional problem, because the ACLs are not
> directly compared -- they get parsed first, and that undoes the
> quoting. It seems a tad fragile, maybe, but perhaps not worth worrying
> about. It's also not really the fault of this patch anyway, but it was
> the only thing I found while looking through this, so I figured I
> would mention it.

Thanks for reviewing and for identifying that. I think this boils down to the
comment being too dire given current use cases. It's important for both
quoteAclUserName() and putid() to quote metacharacters, but it's okay if one
of them quotes non-metacharacters that the other doesn't quote.

I guess the behavior mismatch you've identified also makes this comment wrong:

/*
* Test whether an identifier char can be left unquoted in ACLs.
*
* Formerly, we used isalnum() even on non-ASCII characters, resulting in
* unportable behavior. To ensure dump compatibility with old versions,
* we now treat high-bit-set characters as always requiring quoting during
* putid(), but getid() will always accept them without quotes.
*/
static inline bool
is_safe_acl_char(unsigned char c, bool is_getid)

It's not just "compatibility with old versions" as long as pg_dump calls to
quoteAclUserName() are still churning out affected values.

Best if we make quoteAclUserName() match putid(), so the comments can just be
correct and nobody needs to wonder about the mismatch.