Re: [HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread Tom Lane
kevin brintnall <[EMAIL PROTECTED]> writes:
> Are you suggesting that the pair (reloid,attnum) is superior for
> identifying a pg_attribute entry?

Yes.  It's just as unique, and it makes it easy to see the relationship
between the table and its columns.  Moreover, it's what we're already
using in pg_depend.

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread kevin brintnall
On Fri, Jan 20, 2006 at 07:09:46PM -0500, Tom Lane wrote:
> kevin brintnall <[EMAIL PROTECTED]> writes:
> >  * add OID column to pg_attribute.  This permits dependencies to be
> >registered correctly in pg_shdepend.
> 
> No, no ... the precedent in pg_depend is that columns are represented as
> the table's OID plus a column number.  Please don't invent some random
> other notation for a column, especially not one that is so expensive to
> relate to the parent table.  Add a subobject ID to pg_shdepend instead.

I was referring to the dependency that exists between a grantee and any
pg_attribute ACL entries that mention the grantee.  When the role is
dropped, the ACL entries that mention that role have to be removed.

Specifically, I propose creating an entry such as the following in
pg_shdepend for every grantee G, for every column C in which G is
mentioned:

classid= AttributeRelationId /* 1249 */
objid  = C.oid
refclassid = AuthIdRelationId /* 1260 */
refobjid   = G.oid
deptype= 'a'/* SHARED_DEPENDENCY_ACL */

Are you suggesting that the pair (reloid,attnum) is superior for
identifying a pg_attribute entry?  Are there any other possible uses for
pg_attribute.oid?

> > STILL LEFT TO DO:
> 
> My recollection is that there's quite some deal of code that assumes
> pg_attribute rows are fixed-width.  You will have some issues there.
> It's possible though that none of that code needs to access privileges,
> in which case you'd be OK just dropping off the ACL data from the
> in-memory copies of pg_attribute rows.  Another possible solution is the
> pg_attrdef model, ie, keep the ACLs somewhere else.

I'm employing the same hack^H^H^H^Hmethod that is currently used in
pg_class.

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread Tom Lane
kevin brintnall <[EMAIL PROTECTED]> writes:
>  * add OID column to pg_attribute.  This permits dependencies to be
>registered correctly in pg_shdepend.

No, no ... the precedent in pg_depend is that columns are represented as
the table's OID plus a column number.  Please don't invent some random
other notation for a column, especially not one that is so expensive to
relate to the parent table.  Add a subobject ID to pg_shdepend instead.

> STILL LEFT TO DO:

My recollection is that there's quite some deal of code that assumes
pg_attribute rows are fixed-width.  You will have some issues there.
It's possible though that none of that code needs to access privileges,
in which case you'd be OK just dropping off the ACL data from the
in-memory copies of pg_attribute rows.  Another possible solution is the
pg_attrdef model, ie, keep the ACLs somewhere else.

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


[HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread kevin brintnall
Fellow Hackers,

I've been working on this item for a little while, and I'm starting to see
some code come together.  I wanted to solicit some feedback before I got
too far along to make sure I'm on the right track.

Here's a rough overview of what I've done so far:

-

PARSER:

 * modified parser to accept SQL column privs syntax

 * created a PrivAttr Node which holds ( priv, attr[] ) pairs.  Currently,
   it's just a list of strings.  For example, when you call...

GRANT SELECT, UPDATE (col1, col2) ON table1, table2 to grantee;

   ... the parser creates a list of Nodes:

("select", NIL), ("update", ("col1", "col2"))

SYSTEM CATALOG:

 * add "attacl aclinfo[]" column to pg_attribute table and Form_pg_attribute.
 * add OID column to pg_attribute.  This permits dependencies to be
   registered correctly in pg_shdepend.
 * populated attacl column in existing pg_attribute bootstrap with NULLs
 * allocated an unused oid for each of the pg_attribute rows that are
   bootstrapped
 * created an oid index on pg_attribute

 * modified ExecuteGrantStmt to handle the PrivAttr structure instead of
   the list of strings
 * modified ExecuteGrantStmt to do a nested loop over all
   (column,relation) pairs in the GRANT and find oids for all of the
   attributes.

PSQL COMMAND LINE:

 * display column privileges with  "\d+ table"

STILL LEFT TO DO:

 * implement ExecGrant_Attribute() to modify pg_attribute
 * verify query against column privileges in addition to table privileges
 * register dependencies
 * pg_dump column privileges

-

I'd welcome any feedback on the design changes I've made, or any other
potential snags I should watch out for.

Thanks.

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings