Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread KaiGai Kohei
Tom Lane wrote: > Petr Jelinek writes: >> KaiGai Kohei napsal(a): >>> SELECT INTO does not check ACL_INSERT on the newly created tables, because >>> we had been able to assume the table owner always has privilege to insert >>> values into the new table. >>> So, OpenIntoRel() didn't check this obvi

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Tom Lane
Petr Jelinek writes: > KaiGai Kohei napsal(a): >> SELECT INTO does not check ACL_INSERT on the newly created tables, because >> we had been able to assume the table owner always has privilege to insert >> values into the new table. >> So, OpenIntoRel() didn't check this obvious privilege. >> >> B

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Alvaro Herrera
Petr Jelinek escribió: > Petr Jelinek napsal(a): > >Tom Lane napsal(a): > >>Petr Jelinek writes: > >>>Tom Lane napsal(a): > One thing that seems like it's likely to be an annoyance in practice > is the need to explicitly do DROP OWNED BY to get rid of pg_defaul

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread KaiGai Kohei
Petr Jelinek wrote: > KaiGai Kohei napsal(a): >> I tried to check the default ACL behavior. >> >> It works for me fine, good, but ... >> >> postgres=> SELECT * INTO t3 FROM t1; >> SELECT >> postgres=> SELECT * FROM t3; >>a | b >> ---+- >>1 | aaa >>2 | bbb >> (2 rows) >> >

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Petr Jelinek
Petr Jelinek napsal(a): Tom Lane napsal(a): Petr Jelinek writes: Tom Lane napsal(a): One thing that seems like it's likely to be an annoyance in practice is the need to explicitly do DROP OWNED BY to get rid of pg_default_acl entries for a role to be dropp

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Petr Jelinek
KaiGai Kohei napsal(a): > I tried to check the default ACL behavior. > > It works for me fine, good, but ... > > postgres=> SELECT * INTO t3 FROM t1; > SELECT > postgres=> SELECT * FROM t3; >a | b > ---+- >1 | aaa >2 | bbb > (2 rows) > > postgres=> INSERT INTO t3 VALUES

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread KaiGai Kohei
I tried to check the default ACL behavior. postgres=# \c - ymj psql (8.5devel) You are now connected to database "postgres" as user "ymj". postgres=> ALTER DEFAULT PRIVILEGES REVOKE INSERT ON TABLE FROM ymj; ALTER DEFAULT PRIVILEGES postgres=> CREATE TABLE t2 (x int, y text); CREATE

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Brendan Jurd
2009/10/6 Tom Lane : > Brendan Jurd writes: >> I pulled the latest sources and gave it a whirl.  Things worked as >> expected in psql, but I was a little surprised when I headed into the >> documentation.  The first place I visited was Chapter 20 - Database >> Roles and Privileges, but there was n

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek writes: Tom Lane napsal(a): One thing that seems like it's likely to be an annoyance in practice is the need to explicitly do DROP OWNED BY to get rid of pg_default_acl entries for a role to be dropped. Yeah I am not happy about this either but t

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Brendan Jurd writes: > I pulled the latest sources and gave it a whirl. Things worked as > expected in psql, but I was a little surprised when I headed into the > documentation. The first place I visited was Chapter 20 - Database > Roles and Privileges, but there was no mention of the default AC

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Petr Jelinek writes: > Tom Lane napsal(a): >> One thing that seems like it's likely to be an annoyance in practice >> is the need to explicitly do DROP OWNED BY to get rid of pg_default_acl >> entries for a role to be dropped. > Yeah I am not happy about this either but there is not much we can d

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek writes: [ latest default-ACLs patch ] Applied with a fair amount of editorial polishing. Notably I changed the permissions requirements a bit: Thank you very much Tom. One thing that seems like it's likely to be an annoyance in practice is the

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Brendan Jurd
2009/10/6 Tom Lane : > Applied with a fair amount of editorial polishing.  Notably I changed > the permissions requirements a bit: > Thanks and congratulations! I'm really looking forward to this feature. I pulled the latest sources and gave it a whirl. Things worked as expected in psql, but I

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Josh Berkus writes: > On 10/3/09 8:09 AM, Kevin Grittner wrote: >> Robert Haas wrote: > let's let the default, global default ACL contain the hard-wired > privileges, instead of making them hardwired. > Wow, that would be great. It would meant that DBAs could change the > global default permiss

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Petr Jelinek writes: > [ latest default-ACLs patch ] Applied with a fair amount of editorial polishing. Notably I changed the permissions requirements a bit: * for IN SCHEMA, the *target* role has to have CREATE permission on the target schema. Without this, the command is a bit pointless sinc

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-04 Thread Josh Berkus
On 10/3/09 8:09 AM, Kevin Grittner wrote: > Robert Haas wrote: > > > let's let the default, global default ACL contain the hard-wired >> privileges, instead of making them hardwired. Wow, that would be great. It would meant that DBAs could change the global default permissions. --Josh -- Sen

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-03 Thread Kevin Grittner
Robert Haas wrote: > let's let the default, global default ACL contain the hard-wired > privileges, instead of making them hardwired. +1 -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/p

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-02 Thread Petr Jelinek
Petr Jelinek napsal(a): Robert Haas napsal(a): I'm going to reiterate what I suggested upthread... let's let the default, global default ACL contain the hard-wired privileges, instead of making them hardwired. Then your objects will get those privileges not because they are hard-wired, but bec

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-02 Thread Petr Jelinek
Robert Haas napsal(a): On Thu, Oct 1, 2009 at 1:37 PM, Tom Lane wrote: Petr Jelinek writes: because it seems like merging privileges seems to be acceptable for most (although I am not sure I like it, but I don't have better solution for managing conflicts), I changed the patch to do j

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Robert Haas
On Thu, Oct 1, 2009 at 1:37 PM, Tom Lane wrote: > Petr Jelinek writes: >> because it seems like merging privileges seems to be acceptable for most >> (although I am not sure I like it, but I don't have better solution for >> managing conflicts), I changed the patch to do just that. > > It's not c

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Petr Jelinek
Tom Lane wrote: Stephen Frost writes: Erm, wait, we're going to drop the only piece of this that outside folks have actually been asking for? Specifically, having per-schema default ACLs? They are per-schema for the objects belonging to the granting user. Otherwise you have a bunch o

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Stephen Frost writes: > Erm, wait, we're going to drop the only piece of this that outside folks > have actually been asking for? Specifically, having per-schema default > ACLs? They are per-schema for the objects belonging to the granting user. Otherwise you have a bunch of nasty issues, includ

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > This doesn't actually address the entire problem. How about > > schema-level default grants which you want to override with per-role > > default grants? Or the other way around? Is it always only more > > permissive with more de

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Stephen Frost writes: > This doesn't actually address the entire problem. How about > schema-level default grants which you want to override with per-role > default grants? Or the other way around? Is it always only more > permissive with more defaults? Even when the grantee is the same? Well

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Petr Jelinek writes: > > because it seems like merging privileges seems to be acceptable for most > > (although I am not sure I like it, but I don't have better solution for > > managing conflicts), I changed the patch to do just that. > > It's not clear

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Petr Jelinek writes: > because it seems like merging privileges seems to be acceptable for most > (although I am not sure I like it, but I don't have better solution for > managing conflicts), I changed the patch to do just that. It's not clear to me whether we have consensus on this approach.

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-30 Thread Petr Jelinek
Hi all, because it seems like merging privileges seems to be acceptable for most (although I am not sure I like it, but I don't have better solution for managing conflicts), I changed the patch to do just that. Also I think I addressed all other problems pointed out by Tom. Namely I added pg

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Josh Berkus
Tom, > Hmm ... interesting proposal. Simple to understand and simple to > implement, which are both to the good. I'm not clear though on whether > this behavior would be useful in practice. Any comments from those > who've been asking for default ACLs? I'd be fine with it. My goals here are t

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek writes: That's how it works now actually, the problem is that when you grant something in the chain you can't revoke it anywhere else in the chain when you are merging privileges as you proposed. To allow that, you have to have some notion of a priori

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Robert Haas
On Tue, Sep 29, 2009 at 11:05 AM, Tom Lane wrote: > Petr Jelinek writes: >> That's how it works now actually, the problem is that when you grant >> something in the chain you can't revoke it anywhere else in the chain >> when you are merging privileges as you proposed. > > To allow that, you have

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Tom Lane
Petr Jelinek writes: > That's how it works now actually, the problem is that when you grant > something in the chain you can't revoke it anywhere else in the chain > when you are merging privileges as you proposed. To allow that, you have to have some notion of a priority order among the availa

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Robert Haas napsal(a): On Mon, Sep 28, 2009 at 11:47 PM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: One potential trouble spot is that presumably the built-in default privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate with user-specified defaults.

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Robert Haas
On Mon, Sep 28, 2009 at 11:47 PM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> > One potential trouble spot is that presumably the built-in default >> > privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate >> > with user-specified defaults. >> >> Why not? >

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Tom Lane napsal(a): Especially since we're doing GRANT ALL ON at the same time. That's another patch that has an *excellent* chance of getting rejected on pretty much the same grounds. I don't really see how this applies to GRANT ON ALL. You don't have to predict future there so if y

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Stephen Frost napsal(a): * Robert Haas (robertmh...@gmail.com) wrote: One potential trouble spot is that presumably the built-in default privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate with user-specified defaults. Why not? How would you have a default that s

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Tom Lane napsal(a): I thought we were trying to keep this solution as simple as possible. It's meant to be a simple feature for simple use cases. I know we all love making stuff as ornate and complex as possible around here, but that kind of defeats the purpose of having DefaultACLs, as well as

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > > One potential trouble spot is that presumably the built-in default > > privileges (eg, PUBLIC EXECUTE for functions) would *not* cumulate > > with user-specified defaults. > > Why not? How would you have a default that says "I *don't* want public e

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 10:44 PM, Tom Lane wrote: > Robert Haas writes: >> I haven't read the patch, but it seems like one possible solution to >> this problem would be to declare that any any DEFAULT PRIVILEGES you >> set are cumulative.  If you configure a global default, a per-schema >> defaul

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Robert Haas writes: > I haven't read the patch, but it seems like one possible solution to > this problem would be to declare that any any DEFAULT PRIVILEGES you > set are cumulative. If you configure a global default, a per-schema > default, a default for tables whose names begin with the letter

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 1:32 PM, Tom Lane wrote: > The generic issue that the code doesn't even think about addressing > is which default should apply when there's potentially more than one > applicable default?  As long as there's only global and per-schema > defaults, it's not too hard to decide

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Cathy Mullican
On 9/28/09, Josh Berkus wrote > > > I already mentioned one case that there's longstanding demand for, which > > is to instantiate the correct permissions on new partition child tables. > > Wouldn't that be handled by inheritance? I wish, but no: http://www.postgresql.org/docs/current/interactiv

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus writes: > I think trying to make this patch a panacea in the first iteration is > liable to backfire. I did not suggest any such thing --- the current scope of functionality is fine by me for a first cut. What I *am* saying is that we had better have some idea of how we are going to

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Josh Berkus
Tom, > The owning-ROLE match is required, else you have issues with exactly > what the ACL really means. What we're discussing is what other filters > might exist to determine which objects are affected. The patch already > tries to handle the cases of "all owned objects" and "all owned objects

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus writes: >> But more generally, this is a fairly large and complicated patch in >> comparison to the reward, if the intention is that it will never support >> anything more than the one case of "IN SCHEMA foo" filtering. > I thought we were doing ROLEs? The owning-ROLE match is requir

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Josh Berkus
Tom, >> I thought the idea was to simply avoid that situation. Maybe we want to >> forget about global defaults if that's the case, and just do the ROLE >> defaults. > > That seems like a pretty dead-end design. Well, the whole purpose for DefaultACLs is to simplify administration for the simpl

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus writes: >> This isn't just a matter of a few missed cases while coding, I think. >> The generic issue that the code doesn't even think about addressing >> is which default should apply when there's potentially more than one >> applicable default? > I thought the idea was to simply a

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Josh Berkus
> There is another large problem, too. The patch seems to have > only half-baked support for global defaults (those not tied to a > specific schema) --- it looks like you can put them in, but half > of the code will ignore them or else fail while trying to use them. > This isn't just a matter of

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Petr Jelinek
Tom Lane wrote: Petr Jelinek writes: [ latest version of DefaultACLs patch ] I started looking through this patch, but found that it's not nearly ready to commit :-(. The big missing piece is that there's no pg_dump support for default ACLs. That's a bigger chunk of code than I have

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Petr Jelinek writes: > [ latest version of DefaultACLs patch ] I started looking through this patch, but found that it's not nearly ready to commit :-(. The big missing piece is that there's no pg_dump support for default ACLs. That's a bigger chunk of code than I have time/interest to write, a

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-24 Thread Jan Urbański
Petr Jelinek wrote: > Jan Urbański napsal(a): >> Dependencies suck, I know.. >> > > Cross-database dependencies do. > > I had to make target role owner of the default acls which adds some side > effects like the fact that it blocks DROP ROLE so DROP OWNED BY has to > be used. > As for REASSIGN

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-24 Thread Petr Jelinek
Jan Urbański napsal(a): Dependencies suck, I know.. Cross-database dependencies do. I had to make target role owner of the default acls which adds some side effects like the fact that it blocks DROP ROLE so DROP OWNED BY has to be used. As for REASSIGN OWNED, it does not reassign anything

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-24 Thread Jan Urbański
OK, the previous problem went away, but I can still do something like that: postgres=# create role test; CREATE ROLE postgres=# create role test2; CREATE ROLE postgres=# create database db; CREATE DATABASE postgres=# \c db psql (8.5devel) You are now connected to database "db". db=# alter default

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-24 Thread Petr Jelinek
Jan Urbański napsal(a): Petr Jelinek wrote: I made some more small adjustments - mainly renaming stuff after Tom's comment on anonymous code blocks patch and removed one unused shared dependency. Hi, the patch still has some issues with dependency handling: postgres=# create role tes

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-24 Thread Jan Urbański
Petr Jelinek wrote: > I made some more small adjustments - mainly renaming stuff after Tom's > comment on anonymous code blocks patch and removed one unused shared > dependency. Hi, the patch still has some issues with dependency handling: postgres=# create role test; CREATE ROLE postgres=# crea

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-23 Thread Petr Jelinek
I made some more small adjustments - mainly renaming stuff after Tom's comment on anonymous code blocks patch and removed one unused shared dependency. -- Regards Petr Jelinek (PJMODOS) defacl-2009-09-23.diff.gz Description: Unix tar archive -- Sent via pgsql-hackers mailing list (pgsql-ha

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-22 Thread Petr Jelinek
Jan Urbański napsal(a): Hi, here's a (late, sorry about that) review: Thanks for the comprehensive review! It's unified, not context, but that's trivial. It's not, I have git configured to produce context diffs (see http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_Gi

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-21 Thread Jan Urbański
Hi, here's a (late, sorry about that) review: == Trivia == Patch applies cleanly with a few 1 line offsets. It's unified, not context, but that's trivial. The patch adds some trailing whitespace, which is not good (git diff shows it in red, it's easy to spot it). There's also one hunk that's j

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-18 Thread Petr Jelinek
Jan Urbański napsal(a): Petr Jelinek wrote: So I've been working on solution with which I am happy with (does not mean anybody else will be also though). Hi Petr, I'm reviewing this patch and after reading it I have some comments. Unfortunately, when I got to the compiling part, it tu

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-18 Thread Jan Urbański
Petr Jelinek wrote: > So I've been working on solution with which I am happy with (does not > mean anybody else will be also though). Hi Petr, I'm reviewing this patch and after reading it I have some comments. Unfortunately, when I got to the compiling part, it turned out that the attached patch

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-14 Thread Josh Berkus
Petr, > It's not as easy to do the original idea of setting default privileges > for schema for all users with CREATE privilege on schema but it can > still be done, one just have to update default privileges every time > somebody is granted that privilege, and DBA can still have control over > it

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-14 Thread Petr Jelinek
Josh Berkus wrote: But if I understood Tom's suggestions correctly then his approach does not solve this at all since every one of those users with CREATE TABLE privileges would have to also set same DEFAULT PRIVILEGES and the dba would have no say in the matter. This latter approach benef

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-28 Thread Josh Berkus
Petr, > But if I understood Tom's suggestions correctly then his approach does > not solve this at all since every one of those users with CREATE TABLE > privileges would have to also set same DEFAULT PRIVILEGES and the dba > would have no say in the matter. This latter approach benefits nobody.

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-28 Thread Petr Jelinek
I had some time to work on this patch, and I implemented the ALTER DEFAULT PRIVILEGES syntax as proposed by Tom and adjusted some other stuff, but before I can submit the new patch for commitfest there is still this fundamental issue about how it should behave. The situation is as following. J

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-05 Thread Tom Lane
Petr Jelinek writes: > Tom Lane wrote: >> What I suggest as a way to resolve this last point is that a default ACL >> should apply only to objects owned by the user who creates/modifies the >> default ACL. In this view, the question of which schema the objects are >> in is just an additional filt

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-05 Thread Petr Jelinek
Tom Lane wrote: So my feeling is that adding GRANT ON VIEW is a bad idea. The main argument for doing it seemed to be that the author wanted to be able to grant different default privileges for tables and views, but I'm unconvinced that there's a strong use-case for that. You could very Yes

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-04 Thread Tom Lane
I looked at this patch a bit. I haven't actually read any of the code yet, just reviewed the on-list discussions and the docs, but I think I can make a few comments at the definitional level. First: there's already been some discussion about the VIEW problem. I understand that the patch adds a "G

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-04 Thread Joshua Tolley
On Tue, Aug 04, 2009 at 01:28:00PM -0400, Stephen Frost wrote: > Thanks to Joshua, there weren't really many changes I found for the > docs. Here they are anyway: Yay, I was useful! :) > How about: > > Replaces current privileges with the default privileges, as set using > , for > this object t

Re: [HACKERS] [PATCH] DefaultACLs

2009-08-04 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote: > * Robert Haas (robertmh...@gmail.com) wrote: > > On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinek wrote: > > > The docs are not complete but that's up to Stephen, otherwise the patch > > > should be finished. But I am not the reviewer :) > > > > Well, pe

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-26 Thread Petr Jelinek
Joshua Tolley weote: On Sat, Jul 25, 2009 at 08:41:12PM -0400, Robert Haas wrote: On Sat, Jul 25, 2009 at 8:39 PM, Joshua Tolley wrote: Immediately after concluding I was done with my review, I realized I'd completely forgotten to look at the docs. I've made a few changes based solely o

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 08:41:12PM -0400, Robert Haas wrote: > On Sat, Jul 25, 2009 at 8:39 PM, Joshua Tolley wrote: > > On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote: > >> while writing some basic docs I found bug in dependency handling when > >> doing SET on object type that alread

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Robert Haas
On Sat, Jul 25, 2009 at 8:39 PM, Joshua Tolley wrote: > On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote: >> while writing some basic docs I found bug in dependency handling when >> doing SET on object type that already had some default privileges. >> Attached patch fixes it, it also fi

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote: > while writing some basic docs I found bug in dependency handling when > doing SET on object type that already had some default privileges. > Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT > OPTION behaves

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Robert Haas
On Sat, Jul 25, 2009 at 7:45 PM, Joshua Tolley wrote: > that I wouldn't know what I was talking about. In the meantime, I think this > one is ready to be marked as ... something else. Ready for committer? Sounds right to me. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgr

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 03:50:06PM -0400, Andrew Dunstan wrote: > Joshua Tolley wrote: >> I figured as much. I can't seem to get past this, despite a make distclean. >> Suggestions, anyone? >> > try a fresh checkout and reapply the patch? [ a couple git clean, git reset, make clean, etc. commands

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Andrew Dunstan
Joshua Tolley wrote: I figured as much. I can't seem to get past this, despite a make distclean. Suggestions, anyone? try a fresh checkout and reapply the patch? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: h

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 11:14:19AM +0200, Petr Jelinek wrote: > Joshua Tolley wrote: >> Am I the only one that gets this on make check, with this version (from >> src/test/regress/log/initdb.log): >> >> selecting default shared_buffers ... 32MB >> creating configuration files ... ok >> creating tem

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Petr Jelinek
Joshua Tolley wrote: Am I the only one that gets this on make check, with this version (from src/test/regress/log/initdb.log): selecting default shared_buffers ... 32MB creating configuration files ... ok creating template1 database in /home/josh/devel/pgsrc/pg85/src/test/regress/./tmp_check/da

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-24 Thread Joshua Tolley
On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote: > Hello, > > while writing some basic docs I found bug in dependency handling when > doing SET on object type that already had some default privileges. > Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT > OPTI

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-24 Thread Nikhil Sontakke
Hi, > I'd still like to have opinion from one of the commiters on "the > VIEW problem" which also affects grant on all patch ( see > http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and > I fear "returned with feedback" might prevent that until next commit fest. > > > I see pote

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-23 Thread Petr Jelinek
Peter Eisentraut wrote: On Thursday 23 July 2009 06:26:05 Petr Jelinek wrote: I'd still like to have opinion from one of the commiters on "the VIEW problem" which also affects grant on all patch ( see http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and I fear "returned wit

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-23 Thread Peter Eisentraut
On Thursday 23 July 2009 06:26:05 Petr Jelinek wrote: > I'd still like to have opinion from one of the commiters on "the > VIEW problem" which also affects grant on all patch ( see > http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and > I fear "returned with feedback" might prev

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Nikhil Sontakke
Hi, > >> Anyway, while this patch might not necessary get commited in this commit >> fest, I'd still like to have opinion from one of the commiters on "the VIEW >> problem" which also affects grant on all patch ( see >> http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php ) and I >> f

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinek wrote: > > The docs are not complete but that's up to Stephen, otherwise the patch > > should be finished. But I am not the reviewer :) > > Well, perhaps we had better prod Stephen then, since complete do

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinek wrote: > Robert Haas wrote: >> >> So are these warts fixed in the latest revision of this patch? >> >> http://archives.postgresql.org/pgsql-hackers/2009-07/msg01216.php >> >> I am gathering that this patch is still a bit of a WIP.  I think it >> might

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:21 PM, Joshua Tolley wrote: > On Wed, Jul 22, 2009 at 08:54:19PM -0400, Robert Haas wrote: >> I am gathering that this patch is still a bit of a WIP. > > I don't consider it a WIP. Petr posted a patch a couple of days ago, but I've > not been able to verify its changes or

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Petr Jelinek
Robert Haas wrote: So are these warts fixed in the latest revision of this patch? http://archives.postgresql.org/pgsql-hackers/2009-07/msg01216.php I am gathering that this patch is still a bit of a WIP. I think it might be best to mark it returned with feedback and let Petr resubmit for the n

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Joshua Tolley
On Wed, Jul 22, 2009 at 08:54:19PM -0400, Robert Haas wrote: > I am gathering that this patch is still a bit of a WIP. I don't consider it a WIP. Petr posted a patch a couple of days ago, but I've not been able to verify its changes or perform some additional testing I had in mind, because of my

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Fri, Jul 17, 2009 at 9:46 PM, Joshua Tolley wrote: > On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote: >> Hello, >> >> this is first public version of our DefaultACLs patch as described on >> http://wiki.postgresql.org/wiki/DefaultACL . > > Ok, here's my first crack at a comprehensiv

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-19 Thread Petr Jelinek
Hello, while writing some basic docs I found bug in dependency handling when doing SET on object type that already had some default privileges. Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT OPTION behaves like REVOKE now). And there is also initial version of those ba

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-18 Thread Petr Jelinek
Joshua Tolley wrote: First, as you've already pointed out, this needs documentation. /me looks at Stephen ;) Once I added the missing semicolon mentioned earlier, it applies and builds As we discussed outside of list, my compiler didn't picked that one because I didn't use --enable-cass

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Andrew Dunstan
Joshua Tolley wrote: I don't know how patches that require catalog version changes are generally handled; should the patch include that change? The committer should handle that. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Joshua Tolley
On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote: > Hello, > > this is first public version of our DefaultACLs patch as described on > http://wiki.postgresql.org/wiki/DefaultACL . Ok, here's my first crack at a comprehensive review. There's more I need to look at, eventually. Some of

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Joshua Tolley
On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote: > Hello, > > this is first public version of our DefaultACLs patch as described on > http://wiki.postgresql.org/wiki/DefaultACL . I've been asked to review this. I can't get it to build, because of a missing semicolon on line 1608. I

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Petr Jelinek
Stephen Frost wrote: I agree that they should be consistant. The GRANT ON ALL shares alot more of the syntax with GRANT than DefaultACL though, which makes it a more interesting question there. I can understand not wanting to duplicate the GRANT syntax. I think my suggestion would be to add a

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Stephen Frost
Hey, * Nikhil Sontakke (nikhil.sonta...@enterprisedb.com) wrote: > > We can certainly do it either way, but I don't see much downside to > > having a new enum and a number of downsides with modifying the existing > > grant enums. > > Sure, I understand. But if we want to go the DefaultACLs way, t

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Nikhil Sontakke
Hi, >> I briefly looked at the DefaultACLs patch. Can you not re-use the >> GrantStmt structure for the defaults purpose too? You might have to >> introduce an "is_default" boolean similar to the "is_schema" boolean >> that  you have added in the "GRANT ON ALL" patch. If you think you can >> re-us

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Stephen Frost
Nikhil, * Nikhil Sontakke (nikhil.sonta...@enterprisedb.com) wrote: > I briefly looked at the DefaultACLs patch. Can you not re-use the > GrantStmt structure for the defaults purpose too? You might have to > introduce an "is_default" boolean similar to the "is_schema" boolean > that you have adde

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Petr Jelinek
Nikhil Sontakke wrote: There is however one thing that needs some attention. Both patches add distinction between VIEW and TABLE objects for acls into parser and they both do it differently. GRANT ON ALL works by adding ACL_OBJECT_VIEW and tracks that object type in code (that was my original met

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Nikhil Sontakke
Hi, >> > > No, DefaultACLs applies to objects created in the future while GRANT ON ALL > affects existing objects. I see. > DefaultACLs is more important functionality so it should probably take > precedence in review process. > > There is however one thing that needs some attention. Both patche

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-16 Thread Petr Jelinek
Nikhil Sontakke wrote: Does this new DefaultACL patch nullify this earlier one? Or it is different and should be looked at first since it was added to the commitfest before the defaultACL patch? It is a bit confusing. Please clarify. No, DefaultACLs applies to objects created in the future whi

  1   2   >