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

CVSROOT:    /cvsroot
Module name:    pgsql-server
Changes by:    momjian@svr1.postgresql.org    04/05/02 10:38:28

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:
    Revert patch --- needs more generalized solution.

    > 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

OK, patch reverted at request of Tom and Peter.  Please propose a more
generalitzed soluion.  Thanks.

---------------------------------------------------------------------------

Fabien COELHO wrote:
>
> 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
>

--
  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

> OK, patch reverted at request of Tom and Peter.  Please propose a more
> generalitzed soluion.  Thanks.

Sigh.

You refuse a 10 lines patch to access a stupid opaque type in the backend.
You both refuse some things to be done in the back end, and also to add
what is needed to do it in userland. Moreover, I don't think this patch
did hurt anybody, as it was pretty invisible and was useful to me.

I'm tired. When I'll be too tired, you'll just lose a contributor. A very
small loss indeed, but I don't think it is a good policy for your project.

Have a nice day,

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

Fabien COELHO wrote:

>> OK, patch reverted at request of Tom and Peter.  Please propose a more
>> generalitzed soluion.  Thanks.
>
> Sigh.
>
> You refuse a 10 lines patch to access a stupid opaque type in the backend.
> You both refuse some things to be done in the back end, and also to add
> what is needed to do it in userland. Moreover, I don't think this patch
> did hurt anybody, as it was pretty invisible and was useful to me.

The point where it will hurt is not now, but when or if we need to
change the internal implementation of the entire rights system. Because
at the time we rip out the whole ACL, you or somebody else will ask
absolutely justified for backward compatibility. The reason to keep
things as opaque is the freedom this gives to change implementation
details without asking if it could possibly break existing code.

>
> I'm tired. When I'll be too tired, you'll just lose a contributor. A very
> small loss indeed, but I don't think it is a good policy for your project.

I do understand the frustration, but I hope you understand the larger
scale of problems that would be created if your patch got accepted.


Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

I agree with you that work is needed in the acl area.

> I hope you understand the larger scale of problems that would be created
> if your patch got accepted.

I would be ready to bet that I would be the only user for these functions.
So large looks like me alone;-)

Have a nice day,

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