Re: GRANT ... TABLE for property graph

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
>>> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)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.
>