Re: [PATCH] ALTER DEFAULT PRIVILEGES with GRANT/REVOKE ON SCHEMAS

Hi all,

I noticed that we have no option to set default privileges for newly
created schemas, other than calling GRANT explicitly. At work I use ALTER
DEFAULT PRIVILEGE (ADP) command extensively, as the developers are
permitted to manage DDL on the databases, and all work fine except for when
a new schema is created. So,I'd like to propose this very simple patch
(attached) that adds the capability of using SCHEMAS, adding the following
syntax to ADP:

ALTER DEFAULT PRIVILEGES
[ FOR { ROLE | USER } target_role [, ...] ]
abbreviated_grant_or_revoke

where abbreviated_grant_or_revoke is one of:

GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] }
ON SCHEMAS
TO { [ GROUP ] role_name | PUBLIC } [, ...] [ WITH GRANT OPTION ]

REVOKE [ GRANT OPTION FOR ]
{ USAGE | CREATE | ALL [ PRIVILEGES ] }
ON SCHEMAS
FROM { [ GROUP ] role_name | PUBLIC } [, ...]
[ CASCADE | RESTRICT ]

The patch itself is really straight forward (I'm new to sending patches, so
I've chosen a simple one), and there is only one thing that concerns me (as
in, if I did it right/good). The difference in syntax for SCHEMAS and the
other objects is that IN SCHEMA option makes no sense here (as we don't
have nested schemas), and to solve that I simple added the error "cannot
use IN SCHEMA clause when using GRANT/REVOKE ON SCHEMAS".

Does that look good to you?

Also, should I add translations for that error message in other languages
(I can do that without help of tools for pt_BR) or is that a latter process
in the releasing?

Other than that, I added a few regression tests (similar to others used for
ADP), and patched the documentation (my English is not that good, so I'm
open to suggestions). Anything else I forgot?

While at this, I'd like to ask if you are interested in have all the other
types we have in GRANT/REVOKE for ADP (I myself see few use for that at
work, but the symmetry on those commands seems like a good idea). If you
agree, I can take some time to do the others (looks very simple to do). I
just wonder if that should be done as one patch for each, or just a single
patch for all of them (perhaps send the sequence of patches in order, as
certainly one will conflict with the other if done apart).

Best regards,
--
Matheus de Oliveira

On Tue, Nov 22, 2016 at 08:59:09AM -0200, Matheus de Oliveira wrote:
> Hi all,
>
> I noticed that we have no option to set default privileges for newly
> created schemas, other than calling GRANT explicitly. At work I use ALTER
> DEFAULT PRIVILEGE (ADP) command extensively, as the developers are
> permitted to manage DDL on the databases, and all work fine except for when
> a new schema is created. So,I'd like to propose this very simple patch
> (attached) that adds the capability of using SCHEMAS, adding the following
> syntax to ADP:
>
> ALTER DEFAULT PRIVILEGES
> [ FOR { ROLE | USER } target_role [, ...] ]
> abbreviated_grant_or_revoke
>
> where abbreviated_grant_or_revoke is one of:
>
> GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] }
> ON SCHEMAS
> TO { [ GROUP ] role_name | PUBLIC } [, ...] [ WITH GRANT OPTION ]
>
> REVOKE [ GRANT OPTION FOR ]
> { USAGE | CREATE | ALL [ PRIVILEGES ] }
> ON SCHEMAS
> FROM { [ GROUP ] role_name | PUBLIC } [, ...]
> [ CASCADE | RESTRICT ]

I'd love to have this available.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Just sending the same patch but rebase with current master (it was broken
for gram.y after new commits).

Best regards,

Hi,

> The patch itself is really straight forward (I'm new to sending patches, so
> I've chosen a simple one), and there is only one thing that concerns me (as
> in, if I did it right/good). The difference in syntax for SCHEMAS and the
> other objects is that IN SCHEMA option makes no sense here (as we don't have
> nested schemas), and to solve that I simple added the error "cannot use IN
> SCHEMA clause when using GRANT/REVOKE ON SCHEMAS".
>
> Does that look good to you?

To me, It looks fine.

>
> Also, should I add translations for that error message in other languages (I
> can do that without help of tools for pt_BR) or is that a latter process in
> the releasing?
>

I think you should add it but i am not sure when it is done.

> Other than that, I added a few regression tests (similar to others used for
> ADP), and patched the documentation (my English is not that good, so I'm
> open to suggestions). Anything else I forgot?

You have forgot to change the description section of "ADP". In the
description section you need to mention that privileges for schemas
too can be altered along with other database objects. Other than that,
I feel the patch looks good and has no bug.

--
With Regards,
Ashutosh Sharma.
EnterpriseDB: http://www.enterprisedb.com

On Mon, Jan 9, 2017 at 10:58 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
wrote:

> > Also, should I add translations for that error message in other
> languages (I
> > can do that without help of tools for pt_BR) or is that a latter process
> in
> > the releasing?
> >
>
> I think you should add it but i am not sure when it is done.
>
>
I'll ask one of the guys who work with pt_BR translations (I know him in
person).

> > Other than that, I added a few regression tests (similar to others used
> for
> > ADP), and patched the documentation (my English is not that good, so I'm
> > open to suggestions). Anything else I forgot?
>
> You have forgot to change the description section of "ADP". In the
> description section you need to mention that privileges for schemas
> too can be altered along with other database objects.

Oh... Indeed an oversight, thanks for pointing that out.

> Other than that,
> I feel the patch looks good and has no bug.

Attached a rebased version and with the docs update pointed by Ashutosh
Sharma.

Best regards,
--
Matheus de Oliveira

On 10/01/17 17:33, Matheus de Oliveira wrote:
>
> On Mon, Jan 9, 2017 at 10:58 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com
> <mailto:ashu(dot)coek88(at)gmail(dot)com>> wrote:
>
> > Also, should I add translations for that error message in other languages (I
> > can do that without help of tools for pt_BR) or is that a latter process in
> > the releasing?
> >
>
> I think you should add it but i am not sure when it is done.
>
>
> I'll ask one of the guys who work with pt_BR translations (I know him in
> person).

Translations are not handled by patch author but by translation project
so no need.

>
> Attached a rebased version and with the docs update pointed by Ashutosh
> Sharma.
>

The patch looks good, the only thing I am missing is tab completion
support for psql.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

* Petr Jelinek (petr(dot)jelinek(at)2ndquadrant(dot)com) wrote:
> On 10/01/17 17:33, Matheus de Oliveira wrote:
> >
> > On Mon, Jan 9, 2017 at 10:58 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com
> > <mailto:ashu(dot)coek88(at)gmail(dot)com>> wrote:
> >
> > > Also, should I add translations for that error message in other languages (I
> > > can do that without help of tools for pt_BR) or is that a latter process in
> > > the releasing?
> > >
> >
> > I think you should add it but i am not sure when it is done.
> >
> >
> > I'll ask one of the guys who work with pt_BR translations (I know him in
> > person).
>
> Translations are not handled by patch author but by translation project
> so no need.
>
> >
> > Attached a rebased version and with the docs update pointed by Ashutosh
> > Sharma.
> >
>
> The patch looks good, the only thing I am missing is tab completion
> support for psql.

Awesome, glad to hear it. This is also on my list of patches that I'm
planning to look at, just so folks know.

Thanks!

Stephen

On Thu, Jan 19, 2017 at 9:35 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Awesome, glad to hear it. This is also on my list of patches that I'm
> planning to look at, just so folks know.

There is a patch, no new reviews, so moved to CF 2017-03.
--
Michael

On 1/18/17 7:18 PM, Petr Jelinek wrote:
> On 10/01/17 17:33, Matheus de Oliveira wrote:
>>
>> On Mon, Jan 9, 2017 at 10:58 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com
>> <mailto:ashu(dot)coek88(at)gmail(dot)com>> wrote:
>>
>> > Also, should I add translations for that error message in other languages (I
>> > can do that without help of tools for pt_BR) or is that a latter process in
>> > the releasing?
>> >
>>
>> I think you should add it but i am not sure when it is done.
>>
>>
>> I'll ask one of the guys who work with pt_BR translations (I know him in
>> person).
>
> Translations are not handled by patch author but by translation project
> so no need.
>
>>
>> Attached a rebased version and with the docs update pointed by Ashutosh
>> Sharma.
>>
>
> The patch looks good, the only thing I am missing is tab completion
> support for psql.

It looks like this patch is still waiting on an update for tab
completion in psql.

Do you know when will have that patch ready?

--
-David
david(at)pgmasters(dot)net

Hi Matheus,

On 3/2/17 8:27 AM, David Steele wrote:
> On 1/18/17 7:18 PM, Petr Jelinek wrote:
>>
>> The patch looks good, the only thing I am missing is tab completion
>> support for psql.
>
> It looks like this patch is still waiting on an update for tab
> completion in psql.
>
> Do you know when will have that patch ready?

It's been a while since there was a new patch or any activity on this
thread.

If you need more time to produce a patch, please post an explanation for
the delay and a schedule for the new patch. If no patch or explanation
is is posted by 2017-03-16 AoE I will mark this submission
"Returned with Feedback".

Thanks,
--
-David
david(at)pgmasters(dot)net

On 3/13/17 11:15 AM, David Steele wrote:
> Hi Matheus,
>
> On 3/2/17 8:27 AM, David Steele wrote:
>> On 1/18/17 7:18 PM, Petr Jelinek wrote:
>>>
>>> The patch looks good, the only thing I am missing is tab completion
>>> support for psql.
>>
>> It looks like this patch is still waiting on an update for tab
>> completion in psql.
>>
>> Do you know when will have that patch ready?
>
> It's been a while since there was a new patch or any activity on this
> thread.
>
> If you need more time to produce a patch, please post an explanation for
> the delay and a schedule for the new patch. If no patch or explanation
> is is posted by 2017-03-16 AoE I will mark this submission
> "Returned with Feedback".

I have marked this submission "Returned with Feedback". Please feel
free to resubmit when you have a new version.

--
-David
david(at)pgmasters(dot)net

On Thu, Mar 2, 2017 at 10:27 AM, David Steele <david(at)pgmasters(dot)net> wrote:

> It looks like this patch is still waiting on an update for tab
> completion in psql.

Hi All,

Sorry about the long delay... It was so simple to add it to tab-complete.c
that is a shame I didn't do it before, very sorry about that.

Attached the new version of the patch that is basically the same as
previously with the addition to tab completion for psql and rebased with
master.

Hope it is enough. Thank you all.

--
Matheus de Oliveira

Thank you, pushed

Matheus de Oliveira wrote:
>
> On Thu, Mar 2, 2017 at 10:27 AM, David Steele <david(at)pgmasters(dot)net
> <mailto:david(at)pgmasters(dot)net>> wrote:
>
> It looks like this patch is still waiting on an update for tab
> completion in psql.
>
>
> Hi All,
>
> Sorry about the long delay... It was so simple to add it to tab-complete.c that
> is a shame I didn't do it before, very sorry about that.
>
> Attached the new version of the patch that is basically the same as previously
> with the addition to tab completion for psql and rebased with master.
>
> Hope it is enough. Thank you all.
>
> --
> Matheus de Oliveira
>
>
>
>
>

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/