committed
On 08.07.26 13:37, Ashutosh Bapat wrote:
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
<[email protected]> 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.
