Skip site navigation (1) Skip section navigation (2)

Re: [PATCHES] Users/Groups -> Roles

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCHES] Users/Groups -> Roles
Date: 2005-06-28 17:54:17
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackerspgsql-patches
Stephen Frost <sfrost(at)snowman(dot)net> writes:
> Also, I've been looking through the diff between my tree and what you
> committed to CVS and had a couple comments

>   First, sorry about the gratuitous name changes, it helped me find
>   every place I needed to look at the code and think about if it needed
>   to be changed in some way (ie: Int32GetDatum -> ObjectIdGetDatum,
>   etc).  I had planned on changing some of them back to minimize the
>   patch but kind of ran out of time.

No problem, I figured that was why you'd done it, but changing them back
helped me to understand the patch also ;-)

>   Second, looks like I missed fixing an owner check in pg_proc.c

Got it.  I was wondering if there were more --- might be worth checking
all the superuser() calls.

>   Third, I feel it's incorrect to only allow superuser() to change
>   ownership of objects under a role-based system.

I took that out because it struck me as a likely security hole; we don't
allow non-superuser users to give away objects now, and we shouldn't
allow non-superuser roles to do so either.  Moreover the tests you had
were inconsistent (not same test everyplace).

>   Users must be able to
>   create objects owned by a role they're in (as opposed to owned only
>   by themselves).

This is what SET SESSION AUTHORIZATION/SET ROLE is for, no?  You set the
auth to a role you are allowed to be in, then create the object.  I do
notice that we don't have this yet, but it's surely a required piece of
the puzzle.

>   Fourth, not that I use it, but, it looks like my changes to
>   src/interfaces/ecpg/preproc/preproc.y were lost.  Not sure if that was
>   intentional or not

Yeah, it was.  I leave it to Michael Meskes to sync ecpg with the main
parser; on the occasions where I've tried to do it for him, things
didn't work out well.

>   I do wish ecpg could just
>   be the differences necessary for ecpg and be based off the main parser
>   somehow,

Me too, but I haven't seen a way yet.

>   src/tools/pgindent/pgindent also appears to not have gotten the
>   changes that I made.

That's an automatically generated list; there's no need to edit it.

			regards, tom lane

In response to


pgsql-hackers by date

Next:From: Tom LaneDate: 2005-06-28 17:56:05
Subject: Re: Role syntax (or, SQL99 versus sanity)
Previous:From: Joshua D. DrakeDate: 2005-06-28 17:41:43
Subject: Re: Wierd panic with 7.4.7

pgsql-patches by date

Next:From: Andrew DunstanDate: 2005-06-28 18:28:11
Subject: Re: Problem with dblink regression test
Previous:From: Jim C. NasbyDate: 2005-06-28 16:15:46
Subject: Re: Problem with dblink regression test

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group