Thread: pgsql-server/src backend/utils/adt/acl.c inclu ...

CVSROOT:    /cvsroot
Module name:    pgsql-server
Changes by:    momjian@svr1.postgresql.org    04/04/26 12:06:49

Modified files:
    src/backend/utils/adt: acl.c
    src/include/catalog: catversion.h pg_proc.h
    src/include/utils: acl.h
    src/test/regress/expected: privileges.out
    src/test/regress/sql: privileges.sql

Log message:
    Please find a attached a small patch that adds accessor functions
    for "aclitem" so that it is not an opaque datatype.

    I needed these functions to browse aclitems from user land. I can load
    them when necessary, but it seems to me that these accessors for a
    backend type belong to the backend, so I submit them.

    Fabien Coelho

On 26-Apr-04, at 11:06 AM, Bruce Momjian wrote:
>     Please find a attached a small patch that adds accessor functions
>     for "aclitem" so that it is not an opaque datatype.

Shouldn't this patch include some documentation updates?

-Neil

Neil Conway wrote:
> On 26-Apr-04, at 11:06 AM, Bruce Momjian wrote:
> >     Please find a attached a small patch that adds accessor functions
> >     for "aclitem" so that it is not an opaque datatype.
>
> Shouldn't this patch include some documentation updates?

I talked to the author about this.  Turns out none of our permission
functions have docs.  I figure if folks want to use them, they will use
psql \df to find them.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Neil Conway wrote:
>> Shouldn't this patch include some documentation updates?

> I talked to the author about this.  Turns out none of our permission
> functions have docs.

Really?  What is Table 9.37 in
http://developer.postgresql.org/docs/postgres/functions-misc.html
Arguably these functions do not belong right there, but that's hardly a
reason to think that they do not need documentation.

Personally, though, I think that Peter's original objection was right
on.  We shouldn't be exporting these functions at all; it is right to
treat aclitem as an opaque type.  The problem with allowing computations
on aclitems to occur in client-side code is that we will be locking
ourselves into the present representation of access rights, which is
pretty durn foolish considering that we *know* we need to make changes
in that area pretty soon to move closer to SQL compliance (the whole
users/groups/roles business).  The correct approach is not to export
low-level access and put functionality in the client, but to put the
functionality on the server side where it's convenient to change it
at the same time we reimplement ACLs.

Ergo, my recommendation is to revert this change altogether.  Fabien
should figure out the high-level description of what he wants to know
(at a level similar to has_table_privilege() and its ilk) and propose
server-side functions to implement that.

            regards, tom lane

Tom Lane wrote:
> Ergo, my recommendation is to revert this change altogether.  Fabien
> should figure out the high-level description of what he wants to know
> (at a level similar to has_table_privilege() and its ilk) and propose
> server-side functions to implement that.

I agree with that.

> >     Please find a attached a small patch that adds accessor functions
> >     for "aclitem" so that it is not an opaque datatype.
>
> Shouldn't this patch include some documentation updates?

Well I thought about that, but I wasn't sure were to put it.
There is no documentation at all about the aclitem data type.
I'll submit a few lines anyway.

--
Fabien Coelho - coelho@cri.ensmp.fr

Dear Tom,

> http://developer.postgresql.org/docs/postgres/functions-misc.html
> Arguably these functions do not belong right there, but that's hardly a
> reason to think that they do not need documentation.

Sure. I was planing to add something anyway.

> Personally, though, I think that Peter's original objection was right
> on.  We shouldn't be exporting these functions at all; it is right to
> treat aclitem as an opaque type.

I disagree strongly on this point.

As I stated previously, it should a general principle that all information
about internal configuration should be available from SQL, and better if
in a relationnal form. This is among the "10 rules" of what a relationnal
DB should do, as far as I can remember.

Otherwise, it means that you do not trust SQL and the relationnal DB
as a general tool. As a leading developper in a RDB system, I cannot
believe that;-)

> The problem with allowing computations on aclitems to occur in
> client-side code

I'm developping some "pg_advisor" stuff to check for many things in the
database. YOU decided that all that should not be on the server side.
Fine, I agree. Now if you want to keep things opaque in the server...

> is that we will be locking ourselves into the present representation of
> access rights, which is pretty durn foolish.

I perfectly agree with you on this point;-)

The "pg_hba.conf" code is pretty disappointing. I mean by that low
level, no real internal data structure

The aclitem stuff violates all rules I teach to my student about
sound design: it is a array (no NF1) the same field references keys
in different arrays, a null array means something implicitly, aso.

> considering that we *know* we need to make changes in that area pretty
> soon to move closer to SQL compliance (the whole users/groups/roles
> business).  The correct approach is not to export low-level access and
> put functionality in the client, but to put the functionality on the
> server side where it's convenient to change it at the same time we
> reimplement ACLs.

Well, it would be no big stuff to adapt this.

> Ergo, my recommendation is to revert this change altogether.

You're the boss.

> Fabien should figure out the high-level description of what he wants to
> know (at a level similar to has_table_privilege() and its ilk) and
> propose server-side functions to implement that.

Sure, I did that already.

I built a plpgsql functions to return appropriate relations that I can
query. However this plpgsql needs to access your "opaque" type. I can load
the functions, but they seem to me that they belong to the backend.

"has_*_privileges()" is NOT relationnal as it hides queries, so it does
not really suit queries that want to deal with all possible users/groups
and all possible objects. Moreover, I need access to the raw information
to check for its consistency, not the derived functionnal stuff.

--
Fabien Coelho - coelho@cri.ensmp.fr