Thread

  1. PROPERTY GRAPH pg_dump ACL minimization

    Noah Misch <noah@leadboat.com> — 2026-06-30T02:33:08Z

    pg_dump doesn't do its normal ACL minimization for the new PROPERTY GRAPH
    feature.  Patch attached.  See log message for details.
    
    Most of the patch bulk (modest as it is) exists to keep support for dumping
    from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    a beta is without precedent known to me, so I just erred on the side of not
    breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    be fine with that.
    
    This entails a catversion bump on the v19 branch.  If the master branch
    already has a post-branch catversion bump by then, the two catversions should
    remain distinct.  I'll use yyyymmdd1 for v19 and yyyymmdd2 for master.  That
    feels cleanest to me, since it uses the *2 value where it will be
    shortest-lived.  There's precedent in 20b6847 (master) / e256312 (v15).
    
  2. Re: PROPERTY GRAPH pg_dump ACL minimization

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-30T03:11:01Z

    Noah Misch <noah@leadboat.com> writes:
    > Most of the patch bulk (modest as it is) exists to keep support for dumping
    > from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    > a beta is without precedent known to me, so I just erred on the side of not
    > breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    > be fine with that.
    
    > This entails a catversion bump on the v19 branch.
    
    Those points are not unrelated.  If you bump catversion then beta
    testers must use pg_upgrade to get from beta1 to beta2, so you should
    not drop support for dumping from beta1.
    
    I could agree with dropping that support after beta2, though.  That'd
    imply having to update via beta2 to beta3 or later, but I doubt those
    hardy enough to test beta1 would have a problem with that.
    
    > If the master branch
    > already has a post-branch catversion bump by then, the two catversions should
    > remain distinct.  I'll use yyyymmdd1 for v19 and yyyymmdd2 for master.
    
    Check.
    
    (I didn't read the patch, just responded to your commentary.)
    
    			regards, tom lane
    
    
    
    
  3. Re: PROPERTY GRAPH pg_dump ACL minimization

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-01T16:21:13Z

    Thanks Noah for the patch and the report.
    
    On Tue, Jun 30, 2026 at 8:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >
    > Noah Misch <noah@leadboat.com> writes:
    > > Most of the patch bulk (modest as it is) exists to keep support for dumping
    > > from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    > > a beta is without precedent known to me, so I just erred on the side of not
    > > breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    > > be fine with that.
    >
    > > This entails a catversion bump on the v19 branch.
    >
    > Those points are not unrelated.  If you bump catversion then beta
    > testers must use pg_upgrade to get from beta1 to beta2, so you should
    > not drop support for dumping from beta1.
    >
    > I could agree with dropping that support after beta2, though.  That'd
    > imply having to update via beta2 to beta3 or later, but I doubt those
    > hardy enough to test beta1 would have a problem with that.
    >
    
    Should there be two commits one which will be reverted post beta2 and
    one which will stay after beta2?
    
    > > If the master branch
    > > already has a post-branch catversion bump by then, the two catversions should
    > > remain distinct.  I'll use yyyymmdd1 for v19 and yyyymmdd2 for master.
    >
    > Check.
    >
    > (I didn't read the patch, just responded to your commentary.)
    
    I reviewed the patch.
    
    - tblinfo[i].dacl.acldefault = pg_strdup(PQgetvalue(res, i, i_acldefault));
    + /* acldefault computed below */
    
    Rather than spacially separating the acldefault computation, can we
    just write a function to compute the acldefault for a given relkind
    and owner, and call that function here?
    
    Did you consider writing a test for the same?
    
    Other than that the patch looks good.
    
    I wondered whether we are missing special handling for PROPGRAPH at
    other places. I looked at other places where we handle OBJECT_SEQUENCE
    separately in acl related files. I discovered following missing cases
    
    1. ExecGrant_Relation: I think we should clip the extra privileges
    with a warning when GRANT ... TABLE syntax is used to grant privileges
    on a property graph, just like sequences. To me it looks like we
    should prohibit GRANT ... TABLE on property graph altogether. But
    haven't done so to keep it in sync with sequences. The backward
    compatibility comment,  "For backward compatibility, just ... " should
    not be applicable in case of property graph since we can introduce
    whatever behaviour we expect from GRANT ... TABLE right from the first
    release which introduced property graph. But I am not sure if that's
    the only backward compatibility we are talking about here. Those
    commits go more than a few decades back and commit message itself
    doesn't help me much. Maybe someone with a better historical
    perspective may help. I have also added a test scenario for a
    non-property graph privilege to be added using GRANT ... TABLE syntax.
    
    The second change in this function seems necessary but without it, I
    couldn't find a visible bug. Mostly it's masked because the privileges
    available on a table are a superset of privileges available on a
    property graph.
    
    Now that the function handles property graph separately, its prologue
    needs to change. I think we should mention property graph along with
    sequences as done in the patch OR just mention "all types of objects
    in pg_class."
    
    2. pg_class_aclmask_ext(): this seems to be another omission. Probably
    innocuous since we will test only SELECT privileges on a property
    graph and ignore other default table privileges.
    
    3. pg_default_acl: doesn't need any update since property graph is not
    an object listed in the types of objects for which the user is allowed
    to specify default permissions through pg_default_acl.
    
    All other places which handle OBJECT_SEQUENCE also handle OBJECT_PROPGRAPH.
    
    I also notice that information_schema.pg_propgraph_privileges shows
    only privileges of type "SELECT" so we wouldn't be able to notice a
    privilege type other than SELECT being granted on a property graph
    through information_schema. But a similar filtering exists in the view
    information_schema.table_privileges. So it looks intentional and I
    didn't touch it.
    
    --
    Best Wishes,
    Ashutosh Bapat
    
  4. Re: PROPERTY GRAPH pg_dump ACL minimization

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-03T10:39:20Z

    On Wed, Jul 1, 2026 at 9:51 PM Ashutosh Bapat
    <ashutosh.bapat.oss@gmail.com> wrote:
    >
    > I wondered whether we are missing special handling for PROPGRAPH at
    > other places. I looked at other places where we handle OBJECT_SEQUENCE
    > separately in acl related files. I discovered following missing cases
    >
    > 1. ExecGrant_Relation: I think we should clip the extra privileges
    > with a warning when GRANT ... TABLE syntax is used to grant privileges
    > on a property graph, just like sequences. To me it looks like we
    > should prohibit GRANT ... TABLE on property graph altogether. But
    > haven't done so to keep it in sync with sequences. The backward
    > compatibility comment,  "For backward compatibility, just ... " should
    > not be applicable in case of property graph since we can introduce
    > whatever behaviour we expect from GRANT ... TABLE right from the first
    > release which introduced property graph. But I am not sure if that's
    > the only backward compatibility we are talking about here. Those
    > commits go more than a few decades back and commit message itself
    > doesn't help me much. Maybe someone with a better historical
    > perspective may help. I have also added a test scenario for a
    > non-property graph privilege to be added using GRANT ... TABLE syntax.
    >
    
    Since property graphs share the namespace with regular tables, I think
    GRANT ... TABLE should be supported on property graphs, but restrict
    it to only the privileges applicable to property graphs. Done that way
    in the attached patch.
    
    > The second change in this function seems necessary but without it, I
    > couldn't find a visible bug. Mostly it's masked because the privileges
    > available on a table are a superset of privileges available on a
    > property graph.
    >
    
    This change is needed so that we can provide a correct error message.
    
    Here's a revised patch set.
    0010 is your patch without any changes
    0011 is my changes described above.
    
    
    
    -- 
    Best Wishes,
    Ashutosh Bapat
    
  5. Re: PROPERTY GRAPH pg_dump ACL minimization

    Noah Misch <noah@leadboat.com> — 2026-07-04T14:46:30Z

    On Wed, Jul 01, 2026 at 09:51:13PM +0530, Ashutosh Bapat wrote:
    > On Tue, Jun 30, 2026 at 8:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > Noah Misch <noah@leadboat.com> writes:
    > > > Most of the patch bulk (modest as it is) exists to keep support for dumping
    > > > from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    > > > a beta is without precedent known to me, so I just erred on the side of not
    > > > breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    > > > be fine with that.
    > >
    > > > This entails a catversion bump on the v19 branch.
    > >
    > > Those points are not unrelated.  If you bump catversion then beta
    > > testers must use pg_upgrade to get from beta1 to beta2, so you should
    > > not drop support for dumping from beta1.
    > >
    > > I could agree with dropping that support after beta2, though.  That'd
    > > imply having to update via beta2 to beta3 or later, but I doubt those
    > > hardy enough to test beta1 would have a problem with that.
    
    I agree dropping 19beta1 dump support after beta2 would be reasonable, and I
    hadn't thought of doing so.  However, ...
    
    > Should there be two commits one which will be reverted post beta2 and
    > one which will stay after beta2?
    
    ... since the sole benefit would be removing ~25 lines, I don't think a
    followup patch is worth it.  I wouldn't object if someone wants to remove that
    code later.
    
    > > > If the master branch
    > > > already has a post-branch catversion bump by then, the two catversions should
    > > > remain distinct.  I'll use yyyymmdd1 for v19 and yyyymmdd2 for master.
    > >
    > > Check.
    > >
    > > (I didn't read the patch, just responded to your commentary.)
    > 
    > I reviewed the patch.
    > 
    > - tblinfo[i].dacl.acldefault = pg_strdup(PQgetvalue(res, i, i_acldefault));
    > + /* acldefault computed below */
    > 
    > Rather than spacially separating the acldefault computation, can we
    > just write a function to compute the acldefault for a given relkind
    > and owner, and call that function here?
    
    Such a function would have just one caller, and that would even further
    spatially separate the code lines defining the computation.  Either way is
    fine, but I'm not inclined to change to that.
    
    > Did you consider writing a test for the same?
    
    Somewhat.  Long-term, I think a more general test would have a place.
    
    > Other than that the patch looks good.
    
    Thanks for reviewing.
    
    > I wondered whether we are missing special handling for PROPGRAPH at
    > other places. I looked at other places where we handle OBJECT_SEQUENCE
    > separately in acl related files. I discovered following missing cases
    
    This probably calls for its own thread; feel free to fork the thread for any
    followup on that.
    
    On Fri, Jul 03, 2026 at 04:09:20PM +0530, Ashutosh Bapat wrote:
    > On Wed, Jul 1, 2026 at 9:51 PM Ashutosh Bapat
    > <ashutosh.bapat.oss@gmail.com> wrote:
    > >
    > > I wondered whether we are missing special handling for PROPGRAPH at
    > > other places. I looked at other places where we handle OBJECT_SEQUENCE
    > > separately in acl related files. I discovered following missing cases
    > >
    > > 1. ExecGrant_Relation: I think we should clip the extra privileges
    > > with a warning when GRANT ... TABLE syntax is used to grant privileges
    > > on a property graph, just like sequences. To me it looks like we
    > > should prohibit GRANT ... TABLE on property graph altogether. But
    > > haven't done so to keep it in sync with sequences. The backward
    > > compatibility comment,  "For backward compatibility, just ... " should
    > > not be applicable in case of property graph since we can introduce
    > > whatever behaviour we expect from GRANT ... TABLE right from the first
    > > release which introduced property graph. But I am not sure if that's
    > > the only backward compatibility we are talking about here. Those
    > > commits go more than a few decades back and commit message itself
    > > doesn't help me much. Maybe someone with a better historical
    > > perspective may help. I have also added a test scenario for a
    > > non-property graph privilege to be added using GRANT ... TABLE syntax.
    > 
    > Since property graphs share the namespace with regular tables, I think
    > GRANT ... TABLE should be supported on property graphs, but restrict
    > it to only the privileges applicable to property graphs.
    
    I don't have a strong opinion on that, but I likely would have chosen to block
    GRANT TABLE on a propgraph.  The backward compatibility argument written for
    SEQUENCE likely means that someone noticed GRANT TABLE worked on sequences and
    decided both that it was a mistake and that reversing the mistake would be a
    cure worse than the disease.  There was a rebuttable presumption that when we
    add a new grantable object with its own GRANT subtype, older GRANT subtypes
    should reject that object.
    
    > > 2. pg_class_aclmask_ext(): this seems to be another omission. Probably
    > > innocuous since we will test only SELECT privileges on a property
    > > graph and ignore other default table privileges.
    
    Makes sense.
    
    
    
    
  6. Re: PROPERTY GRAPH pg_dump ACL minimization

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-04T15:51:10Z

    Noah Misch <noah@leadboat.com> writes:
    > On Wed, Jul 01, 2026 at 09:51:13PM +0530, Ashutosh Bapat wrote:
    >> Since property graphs share the namespace with regular tables, I think
    >> GRANT ... TABLE should be supported on property graphs, but restrict
    >> it to only the privileges applicable to property graphs.
    
    > I don't have a strong opinion on that, but I likely would have chosen to block
    > GRANT TABLE on a propgraph.  The backward compatibility argument written for
    > SEQUENCE likely means that someone noticed GRANT TABLE worked on sequences and
    > decided both that it was a mistake and that reversing the mistake would be a
    > cure worse than the disease.
    
    My recollection is that there was an intentional policy change.
    Originally the idea was "why make people be careful about which
    kind of relation they're granting on?".  The arguments made
    against that included:
    
    * It's exposing an implementation detail, namely that sequences
    and tables live in the same catalog.  Admittedly that detail is
    also exposed by the fact that they can't share a name.
    
    * It doesn't comport very well with the fact that the sets of
    possible privileges are different.
    
    * It doesn't obey the SQL standard (I think, maybe someone will
    correct me).
    
    But you are entirely right that we felt that disallowing what used
    to work was worse than leaving it alone.  We need not duplicate that
    mistake for a new kind of relation, and should not.
    
    			regards, tom lane
    
    
    
    
  7. Re: PROPERTY GRAPH pg_dump ACL minimization

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-06T11:54:01Z

    On Sat, Jul 4, 2026 at 8:16 PM Noah Misch <noah@leadboat.com> wrote:
    >
    > On Wed, Jul 01, 2026 at 09:51:13PM +0530, Ashutosh Bapat wrote:
    > > On Tue, Jun 30, 2026 at 8:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > > Noah Misch <noah@leadboat.com> writes:
    > > > > Most of the patch bulk (modest as it is) exists to keep support for dumping
    > > > > from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    > > > > a beta is without precedent known to me, so I just erred on the side of not
    > > > > breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    > > > > be fine with that.
    > > >
    > > > > This entails a catversion bump on the v19 branch.
    > > >
    > > > Those points are not unrelated.  If you bump catversion then beta
    > > > testers must use pg_upgrade to get from beta1 to beta2, so you should
    > > > not drop support for dumping from beta1.
    > > >
    > > > I could agree with dropping that support after beta2, though.  That'd
    > > > imply having to update via beta2 to beta3 or later, but I doubt those
    > > > hardy enough to test beta1 would have a problem with that.
    >
    > I agree dropping 19beta1 dump support after beta2 would be reasonable, and I
    > hadn't thought of doing so.  However, ...
    >
    > > Should there be two commits one which will be reverted post beta2 and
    > > one which will stay after beta2?
    >
    > ... since the sole benefit would be removing ~25 lines, I don't think a
    > followup patch is worth it.  I wouldn't object if someone wants to remove that
    > code later.
    >
    
    As long as the code that needs to be removed after beta2 is clear, I
    am fine. If somebody other than the ones involved here want to remove
    the code, we should make it easy for them to do so.
    
    >
    > > I wondered whether we are missing special handling for PROPGRAPH at
    > > other places. I looked at other places where we handle OBJECT_SEQUENCE
    > > separately in acl related files. I discovered following missing cases
    >
    > This probably calls for its own thread; feel free to fork the thread for any
    > followup on that.
    >
    
    Will do that soon.
    
    -- 
    Best Wishes,
    Ashutosh Bapat
    
    
    
    
  8. Re: PROPERTY GRAPH pg_dump ACL minimization

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-06T11:55:46Z

    On Sat, Jul 4, 2026 at 9:21 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >
    > Noah Misch <noah@leadboat.com> writes:
    > > On Wed, Jul 01, 2026 at 09:51:13PM +0530, Ashutosh Bapat wrote:
    > >> Since property graphs share the namespace with regular tables, I think
    > >> GRANT ... TABLE should be supported on property graphs, but restrict
    > >> it to only the privileges applicable to property graphs.
    >
    > > I don't have a strong opinion on that, but I likely would have chosen to block
    > > GRANT TABLE on a propgraph.  The backward compatibility argument written for
    > > SEQUENCE likely means that someone noticed GRANT TABLE worked on sequences and
    > > decided both that it was a mistake and that reversing the mistake would be a
    > > cure worse than the disease.
    >
    > My recollection is that there was an intentional policy change.
    > Originally the idea was "why make people be careful about which
    > kind of relation they're granting on?".  The arguments made
    > against that included:
    >
    > * It's exposing an implementation detail, namely that sequences
    > and tables live in the same catalog.  Admittedly that detail is
    > also exposed by the fact that they can't share a name.
    >
    > * It doesn't comport very well with the fact that the sets of
    > possible privileges are different.
    >
    > * It doesn't obey the SQL standard (I think, maybe someone will
    > correct me).
    >
    > But you are entirely right that we felt that disallowing what used
    > to work was worse than leaving it alone.  We need not duplicate that
    > mistake for a new kind of relation, and should not.
    
    Thanks for the clarification. I will change the patch to prohibit
    using GRANT ... TABLE on a property graph.
    
    -- 
    Best Wishes,
    Ashutosh Bapat
    
    
    
    
  9. Re: PROPERTY GRAPH pg_dump ACL minimization

    Robert Haas <robertmhaas@gmail.com> — 2026-07-06T16:40:05Z

    On Mon, Jun 29, 2026 at 10:33 PM Noah Misch <noah@leadboat.com> wrote:
    > pg_dump doesn't do its normal ACL minimization for the new PROPERTY GRAPH
    > feature.  Patch attached.  See log message for details.
    >
    > Most of the patch bulk (modest as it is) exists to keep support for dumping
    > from beta1.  I'm not sure whether it was worth bothering.  Breaking dump from
    > a beta is without precedent known to me, so I just erred on the side of not
    > breaking it.  If we were to decide pg_dump could drop support for betas, I'd
    > be fine with that.
    
    Thanks for catching this, Noah.
    
    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.
    
    --
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  10. Re: PROPERTY GRAPH pg_dump ACL minimization

    Noah Misch <noah@leadboat.com> — 2026-07-06T20:44:42Z

    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@leadboat.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.
    
    
    
    
  11. GRANT ... TABLE for property graph

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-08T11:37:10Z

    Starting a new thread from [1] as suggested.
    
    >
    > On Fri, Jul 03, 2026 at 04:09:20PM +0530, Ashutosh Bapat wrote:
    > > On Wed, Jul 1, 2026 at 9:51 PM Ashutosh Bapat
    > > <ashutosh.bapat.oss@gmail.com> wrote:
    > > >
    > > > I wondered whether we are missing special handling for PROPGRAPH at
    > > > other places. I looked at other places where we handle OBJECT_SEQUENCE
    > > > separately in acl related files. I discovered following missing cases
    > > >
    > > > 1. ExecGrant_Relation: I think we should clip the extra privileges
    > > > with a warning when GRANT ... TABLE syntax is used to grant privileges
    > > > on a property graph, just like sequences. To me it looks like we
    > > > should prohibit GRANT ... TABLE on property graph altogether. But
    > > > haven't done so to keep it in sync with sequences. The backward
    > > > compatibility comment,  "For backward compatibility, just ... " should
    > > > not be applicable in case of property graph since we can introduce
    > > > whatever behaviour we expect from GRANT ... TABLE right from the first
    > > > release which introduced property graph. But I am not sure if that's
    > > > the only backward compatibility we are talking about here. Those
    > > > commits go more than a few decades back and commit message itself
    > > > doesn't help me much. Maybe someone with a better historical
    > > > perspective may help. I have also added a test scenario for a
    > > > non-property graph privilege to be added using GRANT ... TABLE syntax.
    > >
    > > Since property graphs share the namespace with regular tables, I think
    > > GRANT ... TABLE should be supported on property graphs, but restrict
    > > it to only the privileges applicable to property graphs.
    >
    > I don't have a strong opinion on that, but I likely would have chosen to block
    > GRANT TABLE on a propgraph.  The backward compatibility argument written for
    > SEQUENCE likely means that someone noticed GRANT TABLE worked on sequences and
    > decided both that it was a mistake and that reversing the mistake would be a
    > cure worse than the disease.  There was a rebuttable presumption that when we
    > add a new grantable object with its own GRANT subtype, older GRANT subtypes
    > should reject that object.
    >
    > > > 2. pg_class_aclmask_ext(): this seems to be another omission. Probably
    > > > innocuous since we will test only SELECT privileges on a property
    > > > graph and ignore other default table privileges.
    >
    > Makes sense.
    
    Modified the patch to disallow GRANT ... TABLE on property graph.
    
    The patch uses errmsg("\"%s\" is a property graph",
    NameStr(pg_class_tuple->relname)) inline with other prohibited cases.
    Additionally it gives errhint("Use GRANT ... ON PROPERTY GRAPH
    instead.")), which may be obvious from the error message, but I
    thought clarity is better than brevity.
    
    There are some cases where we report ""%s" is not a sequence". Inline
    with that we could use errmsg("\"%s\" is not a table",
    NameStr(pg_class_tuple->relname)) with errhint("Use GRANT ... ON
    PROPERTY GRAPH instead.")).
    
    I prefer the first one, but I am ok with the second option as well.
    
    -- 
    Best Wishes,
    Ashutosh Bapat