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 shou

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 operati

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 we

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

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

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

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 tra

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 def

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:

Re: [HACKERS] superuser() shortcuts

2014-10-01 Thread Brightwell, Adam
Stephen, We, as a community, have gotten flak from time-to-time about the > superuser role. We also tend to wish to avoid unnecessary > optimization as it complicates the code base and makes folks reviewing > the code wonder at the exceptions. > I have attached a patch for consideration/

Re: [HACKERS] superuser() shortcuts

2014-10-03 Thread Brightwell, Adam
Stephen, Thanks! Please add it to the next commitfest. Sounds good. I'll update the patch and add accordingly. > I don't think has_rolinherit or has_rolcatupdate really need to move and > it seems unlikely that they'd be needed from elsewhere.. Is there a > reason you think they'd need to b

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 a/src/backend/ac

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

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. https://commitfest.postgresql.org/action/pat

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

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), + er

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 val

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 hav

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

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?

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 >

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 E