Re: [PATCH] DefaultACLs

From: Nikhil Sontakke <nikhil(dot)sontakke(at)enterprisedb(dot)com>
To: Petr Jelinek <pjmodos(at)pjmodos(dot)net>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Joshua Tolley <eggyknap(at)gmail(dot)com>
Subject: Re: [PATCH] DefaultACLs
Date: 2009-07-24 08:45:49
Message-ID: a301bfd90907240145i5f09d251oac8689f1e6d2b681@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

> I'd still like to have opinion from one of the commiters on "the
> VIEW problem" which also affects grant on all patch ( see
> http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and
> I fear "returned with feedback" might prevent that until next commit fest.
>
>
> I see potential for confusion in that GRANT ON TABLE x works if x is a base
> table or a view, but GRANT ON ALL TABLES would not affect views. Maybe you
> need to make up a different syntax to affect only base tables, e.g., GRANT
> ON
> ALL BASE TABLES.
>
>
> That's not what I mean the problem is what is the best way of handling the
> views in implementation itself (there were IIRC 3 possible solutions devised
> and I don't think we have consensus on which is better).

Peter is raising a good question here and it's not related to the
implementation. What he is saying is that in your new implementation
if "GRANT ON ALL TABLES" is invoked, it will affect only RELKIND_TABLE
objects. Whereas the GRANT ON TABLE affects both RELKIND_TABLE and
RELKIND_VIEW types of objects (with and without your patch).

We could have brought in the differentiation with this patch to treat
views and tables separately. So a GRANT ON TABLE would just affect
tables. But I guess that will break existing user scripts which assume
it works against VIEWS too.

I don't know how acceptable the "ON ALL BASE TABLES" sounds to all.

Regards,
Nikhils

> In short,
> 1. add ACL_OBJECT_VIEW into GrantObjectType enum and track that inside code
> 2. create new enum with table, view, function and sequence objects in it
> (that works well for DefaultACLs but not for GRANT ON ALL)
> 3. add some boolean into GrantStmt that would indicate that relation is a
> view (that works for GRANT ON ALL but does not solve anything for
> DefaultACLs)
>
> Currently DefaultACLs patch uses method 2 (because Stephen does not like
> method 1) and GRANT ON ALL patch uses method 1 and it might be better if
> both patches uses only one of those.
> If we went with method 1 we probably should just ditch GrantObjectType
> alltogether and work with subset of ObjectType as other commands do (I
> haven't found any reason for GrantObjectType to exist other than having
> single object type for both TABLE and VIEW).
> And If we choose not to use method 1 then we should probably go with 2 for
> DefaultACLs and 3 for GRANT ON ALL. That is unless somebody has a better
> solution.
>
> --
> Regards
> Petr Jelinek (PJMODOS)

--
http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David E. Wheeler 2009-07-24 09:15:56 Re: When is a record NULL?
Previous Message Peter Eisentraut 2009-07-24 08:21:19 Re: WIP: plpython3