Re: [HACKERS] API change advice: Passing plan invalidation info from the rewriter into the planner?

2014-06-10 Thread Brightwell, Adam
Hey Tom, Hm ... I'm not following why we'd need a special case for superusers and not anyone else? Seems like any useful RLS scheme is going to require more privilege levels than just superuser and not-superuser. As it stands right now, superuser is the only case where RLS policies should

Re: [HACKERS] API change advice: Passing plan invalidation info from the rewriter into the planner?

2014-06-17 Thread Brightwell, Adam
Robert, However, I believe that there has been a lack of focus in the development of the patch thus far in a couple of key areas - first in terms of articulating how it is different from and better than a writeable security barrier view, and second on how to manage the security and

Re: [HACKERS] RLS Design

2014-07-16 Thread Brightwell, Adam
Tom, Thanks for the feedback. 20MB messages to the list aren't that friendly. Please don't do that again, unless asked to. Apologies, I didn't realize it was so large until after it was sent. At any rate, it won't happen again. FWIW, the above syntax is a nonstarter, at least unless

Re: [HACKERS] RLS Design

2014-07-16 Thread Brightwell, Adam
Stephen, Yeah, now that we're trying to bake this into ALTER TABLE we need to be a bit more cautious. I'd think: ALTER TABLE tab POLICY ADD ... Would work though? (note: haven't looked/tested myself) Yes, I just tested it and the following would work from a grammar perspective: ALTER

Re: [HACKERS] RLS Design

2014-07-18 Thread Brightwell, Adam
I think we do want a way to modify policies. However, we tend to avoid syntax that involves unnatural word order, as this certainly does. Maybe it's better to follow the example of CREATE RULE and CREATE TRIGGER and do something this instead: CREATE POLICY policy_name ON table_name USING

[HACKERS] New Model For Role Attributes and Fine Grained Permssions

2014-08-18 Thread Brightwell, Adam
Hi All, This is a proof-of-concept patch for a new model around role attributes and fine grained permissions meant to alleviate the current over dependence on superuser. This is not yet complete and only serves as a proof-of-concept at this point, but I wanted to share it in the hopes of

Re: [HACKERS] Selectivity estimation for inet operators

2014-09-16 Thread Brightwell, Adam
New version with semi join estimation function attached. I have performed the following initial review: - Patch format. -- submitted as unified, but not sure it makes it any easier to read than context format. - Apply to current master (77e65bf). -- success (though, I do get Stripping

Re: [HACKERS] replicating DROP commands across servers

2014-09-16 Thread Brightwell, Adam
I think there's been some changes to this patch since july, care to resend a new version? Sure, here it is. The only difference with the previous version is that it now also supports column defaults. This was found to be a problem when you drop a sequence that some column default

Re: [HACKERS] RLS Design

2014-09-19 Thread Brightwell, Adam
Thom, Also, I seem to get an error message with the following: # create policy nice_colours ON colours for all to joe using (visible = true) with check (name in ('blue','green','yellow')); CREATE POLICY \c - joe insert into colours (name, visible) values ('blue',false); ERROR: function

Re: [HACKERS] superuser() shortcuts

2014-10-03 Thread Brightwell, Adam
All, Thanks! Please add it to the next commitfest. Sounds good. I'll update the patch and add accordingly. Attached is an updated patch. -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com diff --git

[HACKERS] Directory/File Access Permissions for COPY and Generic File Access Functions

2014-10-15 Thread Brightwell, Adam
All, The attached patch for review implements a directory permission system that allows for providing a directory read/write capability to directories for COPY TO/FROM and Generic File Access Functions to non-superusers. This is not a complete solution as it does not currently contain

Re: [HACKERS] Review of GetUserId() Usage

2014-10-16 Thread Brightwell, Adam
I'll break them into three pieces- superuser() cleanup, GetUserId() - has_privs_of_role(), and the additional-role-attributes patch will just depend on the others. The superuser() cleanup has already been submitted to the current commitfest.

Re: [HACKERS] Review of GetUserId() Usage

2014-10-18 Thread Brightwell, Adam
All, I'll break them into three pieces- superuser() cleanup, GetUserId() - has_privs_of_role(), and the additional-role-attributes patch will just depend on the others. Attached is a patch for the GetUserId() - has_privs_of_role() cleanup for review. -Adam -- Adam Brightwell -

Re: [HACKERS] alter user/role CURRENT_USER

2014-10-20 Thread Brightwell, Adam
Kyotaro, Food for thought. Couldn't you reduce the following block: + if (strcmp(stmt-role, current_user) == 0) + { + roleid = GetUserId(); + tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), +

Re: [HACKERS] [TODO] Track number of files ready to be archived in pg_stat_archiver

2014-10-20 Thread Brightwell, Adam
Julien, The following is an initial review: * Applies cleanly to master (f330a6d). * Regression tests updated and pass, including 'check-world'. * Documentation updated and builds successfully. * Might want to consider replacing the following magic number with a constant or perhaps calculated

Re: [HACKERS] [TODO] Track number of files ready to be archived in pg_stat_archiver

2014-10-21 Thread Brightwell, Adam
Julien, Actually, I used the same loop as the archiver one (see backend/postmaster/pgarch.c, function pgarch_readyXlog) to get the exact same number of files. Ah, I see. If we change it in this patch, it would be better to change it everywhere. What do you think ? Hmm... I'd have to

Re: [HACKERS] Directory/File Access Permissions for COPY and Generic File Access Functions

2014-10-21 Thread Brightwell, Adam
Peter, You patch is missing the files src/include/catalog/pg_diralias.h, src/include/commands/diralias.h, and src/backend/commands/diralias.c. (Hint: git add -N) Yikes, sorry about that, not sure how that happened. Attached is an updated patch. -Adam -- Adam Brightwell -

Re: [HACKERS] superuser() shortcuts

2014-10-23 Thread Brightwell, Adam
Alvaro, I noticed something strange while perusing this patch, but the issue predates the patch. Some messages say must be superuser or replication role to foo, but our longstanding practice is to say permission denied to foo. Why do we have this inconsistency? Should we remove it? If we

Re: [HACKERS] superuser() shortcuts

2014-10-23 Thread Brightwell, Adam
I noticed something strange while perusing this patch, but the issue predates the patch. Some messages say must be superuser or replication role to foo, but our longstanding practice is to say permission denied to foo. Why do we have this inconsistency? Should we remove it? If we do want

Re: [HACKERS] security barrier INSERT

2014-10-24 Thread Brightwell, Adam
Drew, IMHO this is nonintuitive, the intuitive behavior of a security_barrier view should be to forbid inserting rows that can’t appear in the view. Isn't that what WITH CHECK OPTION is meant to accomplish? -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database