Re: [PATCHES] Extending grant insert on tables to sequences

2008-07-08 Thread Jaime Casanova
On 7/8/08, Alvaro Herrera <[EMAIL PROTECTED]> wrote:
> Jaime Casanova escribió:
> > On Thu, May 22, 2008 at 1:18 PM, Jaime Casanova <[EMAIL PROTECTED]> wrote:
> > > Hi,
> > >
> > > The idea of this patch is to avoid the need to make explicit grants on
> > > sequences owned by tables.
> >
> > I've noted that the patch i attached is an older version that doesn't
> > compile because of a typo...
> > Re-attaching right patch and fix documentation to indicate the new 
> > behaviour...
>
> I had a look at this patch and it looks good.  The only thing that's not
> clear to me is whether we have agreed we want this to be the default
> behavior?
>

mmm... i don't remember from where i took the equivalences...
i will review if there is any concensus in that...
anyway now i when people should speak about it...

>
> Wouldn't it be clearer to build a list with all the sequences owned by
> the tables in istmt.objects, and then call ExecGrantStmt_oids() a single
> time with the big list?
>

at night i will see the code for this...

-- 
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 87171157

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-07-08 Thread Alvaro Herrera
Jaime Casanova escribió:
> On Thu, May 22, 2008 at 1:18 PM, Jaime Casanova <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > The idea of this patch is to avoid the need to make explicit grants on
> > sequences owned by tables.
> 
> I've noted that the patch i attached is an older version that doesn't
> compile because of a typo...
> Re-attaching right patch and fix documentation to indicate the new 
> behaviour...

I had a look at this patch and it looks good.  The only thing that's not
clear to me is whether we have agreed we want this to be the default
behavior?

A quibble:

> + foreach(cell, istmt.objects)
> + {
> + [...]
> + 
> + istmt_seq.objects = getOwnedSequences(lfirst_oid(cell));
> + if (istmt_seq.objects != NIL)
> + {
> + if (istmt.privileges & (ACL_INSERT)) 
> + istmt_seq.privileges |= ACL_USAGE;
> + else if (istmt.privileges & (ACL_UPDATE)) 
> + istmt_seq.privileges |= ACL_UPDATE;
> + else if (istmt.privileges & (ACL_SELECT)) 
> + istmt_seq.privileges |= ACL_SELECT;
> + 
> + ExecGrantStmt_oids(&istmt_seq);
> + }

Wouldn't it be clearer to build a list with all the sequences owned by
the tables in istmt.objects, and then call ExecGrantStmt_oids() a single
time with the big list?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-05-29 Thread Jaime Casanova
On 5/29/08, Robert Treat <[EMAIL PROTECTED]> wrote:
> On Saturday 24 May 2008 01:19:05 Jaime Casanova wrote:
> > On Sat, May 24, 2008 at 12:09 AM, Alvaro Herrera
> >
> > <[EMAIL PROTECTED]> wrote:
> > > Please add the patch to the commitfest page,
> >
> > Ah! I forgot we have a new process now... patch added to the commitfest
> > page...
> >
>
> What's the use case for extending SELECT on table to SELECT on sequence ?
>

Just to be consistent


-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 087171157

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-05-29 Thread Robert Treat
On Saturday 24 May 2008 01:19:05 Jaime Casanova wrote:
> On Sat, May 24, 2008 at 12:09 AM, Alvaro Herrera
>
> <[EMAIL PROTECTED]> wrote:
> > Please add the patch to the commitfest page,
>
> Ah! I forgot we have a new process now... patch added to the commitfest
> page...
>

What's the use case for extending SELECT on table to SELECT on sequence ?  

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-05-23 Thread Alvaro Herrera
Jaime Casanova escribió:
> On Thu, May 22, 2008 at 1:18 PM, Jaime Casanova <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > The idea of this patch is to avoid the need to make explicit grants on
> > sequences owned by tables.
> >
> 
> I've noted that the patch i attached is an older version that doesn't
> compile because of a typo...
> Re-attaching right patch and fix documentation to indicate the new 
> behaviour...

Please add the patch to the commitfest page,

http://wiki.postgresql.org/wiki/CommitFest:July

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-05-23 Thread Jaime Casanova
On Sat, May 24, 2008 at 12:09 AM, Alvaro Herrera
<[EMAIL PROTECTED]> wrote:
>
> Please add the patch to the commitfest page,
>

Ah! I forgot we have a new process now... patch added to the commitfest page...

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 087171157

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] Extending grant insert on tables to sequences

2008-05-23 Thread Jaime Casanova
On Thu, May 22, 2008 at 1:18 PM, Jaime Casanova <[EMAIL PROTECTED]> wrote:
> Hi,
>
> The idea of this patch is to avoid the need to make explicit grants on
> sequences owned by tables.
>

I've noted that the patch i attached is an older version that doesn't
compile because of a typo...
Re-attaching right patch and fix documentation to indicate the new behaviour...

we need an user visible message to indicate this implicit grant on the
sequences?

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Guayaquil - Ecuador
Cel. (593) 087171157
Index: doc/src/sgml/ref/grant.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.68
diff -c -r1.68 grant.sgml
*** doc/src/sgml/ref/grant.sgml 5 May 2008 01:21:03 -   1.68
--- doc/src/sgml/ref/grant.sgml 24 May 2008 04:46:36 -
***
*** 387,396 
 
  
 
! Granting permission on a table does not automatically extend 
! permissions to any sequences used by the table, including 
! sequences tied to SERIAL columns.  Permissions on 
! sequence must be set separately.
 
  
 
--- 387,395 
 
  
 
! Granting permission on a table automatically extend 
! permissions to any sequences owned by the table, including 
! sequences tied to SERIAL columns.
 
  
 
Index: src/backend/catalog/aclchk.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.146
diff -c -r1.146 aclchk.c
*** src/backend/catalog/aclchk.c12 May 2008 00:00:46 -  1.146
--- src/backend/catalog/aclchk.c24 May 2008 04:46:45 -
***
*** 360,365 
--- 360,402 
}
  
ExecGrantStmt_oids(&istmt);
+ 
+   /*
+* If the objtype is a relation and the privileges includes INSERT, 
UPDATE 
+* or SELECT then extends the GRANT/REVOKE to the sequences owned by the 
+* relation
+*/
+   if ((istmt.objtype == ACL_OBJECT_RELATION) && 
+   (istmt.privileges & (ACL_INSERT | ACL_UPDATE | ACL_SELECT))) 
+   {
+   AclMode priv; 
+   foreach(cell, istmt.objects)
+   {
+   InternalGrant istmt_seq;
+ 
+   istmt_seq.is_grant = istmt.is_grant;
+   istmt_seq.objtype = ACL_OBJECT_SEQUENCE;
+   istmt_seq.grantees = istmt.grantees;
+   istmt_seq.grant_option = istmt.grant_option;
+   istmt_seq.behavior = istmt.behavior;
+ 
+   istmt_seq.all_privs = false;
+   istmt_seq.privileges = ACL_NO_RIGHTS;
+ 
+   istmt_seq.objects = getOwnedSequences(lfirst_oid(cell));
+   if (istmt_seq.objects != NIL)
+   {
+   if (istmt.privileges & (ACL_INSERT)) 
+   istmt_seq.privileges |= ACL_USAGE;
+   else if (istmt.privileges & (ACL_UPDATE)) 
+   istmt_seq.privileges |= ACL_UPDATE;
+   else if (istmt.privileges & (ACL_SELECT)) 
+   istmt_seq.privileges |= ACL_SELECT;
+ 
+   ExecGrantStmt_oids(&istmt_seq);
+   }
+   }
+   } 
  }
  
  /*
Index: src/test/regress/expected/dependency.out
===
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/dependency.out,v
retrieving revision 1.6
diff -c -r1.6 dependency.out
*** src/test/regress/expected/dependency.out5 May 2008 01:21:03 -   
1.6
--- src/test/regress/expected/dependency.out24 May 2008 04:46:59 -
***
*** 16,22 
  DETAIL:  access to table deptest
  DROP GROUP regression_group;
  ERROR:  role "regression_group" cannot be dropped because some objects depend 
on it
! DETAIL:  access to table deptest
  -- if we revoke the privileges we can drop the group
  REVOKE SELECT ON deptest FROM GROUP regression_group;
  DROP GROUP regression_group;
--- 16,23 
  DETAIL:  access to table deptest
  DROP GROUP regression_group;
  ERROR:  role "regression_group" cannot be dropped because some objects depend 
on it
! DETAIL:  access to sequence deptest_f1_seq
! access to table deptest
  -- if we revoke the privileges we can drop the group
  REVOKE SELECT ON deptest FROM GROUP regression_group;
  DROP GROUP regression_group;

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches