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 (3,'ccc');

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Petr Jelinek
Petr Jelinek napsal(a): Tom Lane napsal(a): Petr Jelinek pjmo...@pjmodos.net mailto:pjmo...@pjmodos.net 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

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) postgres= INSERT INTO t3

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Alvaro Herrera
Petr Jelinek escribió: Petr Jelinek napsal(a): Tom Lane napsal(a): Petr Jelinek pjmo...@pjmodos.net mailto:pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-06 Thread KaiGai Kohei
Tom Lane wrote: Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes: On 10/3/09 8:09 AM, Kevin Grittner wrote: Robert Haas robertmh...@gmail.com 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Brendan Jurd
2009/10/6 Tom Lane t...@sss.pgh.pa.us: 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Tom Lane
Brendan Jurd dire...@gmail.com 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-05 Thread Brendan Jurd
2009/10/6 Tom Lane t...@sss.pgh.pa.us: Brendan Jurd dire...@gmail.com 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

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 TABLE

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-04 Thread Josh Berkus
On 10/3/09 8:09 AM, Kevin Grittner wrote: Robert Haas robertmh...@gmail.com 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.

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-03 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com 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:

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 t...@sss.pgh.pa.us wrote: Petr Jelinek pjmo...@pjmodos.net 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

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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Petr Jelinek pjmo...@pjmodos.net 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.

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Stephen Frost sfr...@snowman.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Tom Lane
Stephen Frost sfr...@snowman.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Petr Jelinek
Tom Lane wrote: Stephen Frost sfr...@snowman.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-10-01 Thread Robert Haas
On Thu, Oct 1, 2009 at 1:37 PM, Tom Lane t...@sss.pgh.pa.us wrote: Petr Jelinek pjmo...@pjmodos.net 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

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

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

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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Robert Haas
On Mon, Sep 28, 2009 at 11:47 PM, Stephen Frost sfr...@snowman.net 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

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 sfr...@snowman.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Robert Haas
On Tue, Sep 29, 2009 at 11:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: Petr Jelinek pjmo...@pjmodos.net 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-29 Thread Petr Jelinek
Tom Lane napsal(a): Petr Jelinek pjmo...@pjmodos.net 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

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 to

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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 Petr Jelinek
Tom Lane wrote: Petr Jelinek pjmo...@pjmodos.net 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

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 a

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com 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

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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com 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

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 in

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Josh Berkus j...@agliodbs.com 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Cathy Mullican
On 9/28/09, Josh Berkus j...@agliodbs.com 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:

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 1:32 PM, Tom Lane t...@sss.pgh.pa.us 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Tom Lane
Robert Haas robertmh...@gmail.com 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-09-28 Thread Robert Haas
On Mon, Sep 28, 2009 at 10:44 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com 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

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 execute

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=# create

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

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

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 OWNED, it

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

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

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

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

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

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

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

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-05 Thread Tom Lane
Petr Jelinek pjmo...@pjmodos.net 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

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 Jelinekpjmo...@pjmodos.net wrote: The docs are not complete but that's up to Stephen, otherwise the patch should be finished. But I am not the reviewer :)

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 xref

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

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 Tolleyeggyk...@gmail.com 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

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

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 template1

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:

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 Robert Haas
On Sat, Jul 25, 2009 at 7:45 PM, Joshua Tolleyeggyk...@gmail.com 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

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 8:39 PM, Joshua Tolleyeggyk...@gmail.com 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

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 Tolleyeggyk...@gmail.com 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

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

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 OPTION

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-23 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 fear returned

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 prevent

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 with

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Fri, Jul 17, 2009 at 9:46 PM, Joshua Tolleyeggyk...@gmail.com 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

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

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:21 PM, Joshua Tolleyeggyk...@gmail.com 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

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Robert Haas
On Wed, Jul 22, 2009 at 11:26 PM, Petr Jelinekpjmo...@pjmodos.net 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

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 Jelinekpjmo...@pjmodos.net 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,

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

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

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

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

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

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-use the GrantStmt

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, then we

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

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

Re: [HACKERS] [PATCH] DefaultACLs

2009-07-16 Thread Nikhil Sontakke
Hi Petr, this is first public version of our DefaultACLs patch as described on http://wiki.postgresql.org/wiki/DefaultACL . I have been assigned by Robert to do an initial review of your GRANT ON ALL patch mentioned here (http://archives.postgresql.org/pgsql-hackers/2009-07/msg00207.php) Does

  1   2   >