public inbox for [email protected]  

help / color / mirror / Atom feed

From: Noah Misch <[email protected]>
To: Robert Haas <[email protected]>
Cc: [email protected]
Subject: Re: PROPERTY GRAPH pg_dump ACL minimization
Date: Mon, 6 Jul 2026 13:44:42 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoY7QztkjrzhxgxzSMFZAnF78-JtXy3Vu2-FJVggKM7vUg@mail.gmail.com>
References: <[email protected]>
	<CA+TgmoY7QztkjrzhxgxzSMFZAnF78-JtXy3Vu2-FJVggKM7vUg@mail.gmail.com>

On Mon, Jul 06, 2026 at 12:40:05PM -0400, Robert Haas wrote:
> On Mon, Jun 29, 2026 at 10:33 PM Noah Misch <[email protected]> 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.

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: PROPERTY GRAPH pg_dump ACL minimization
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox