Thread: GRANT USAGE ON SEQUENCE missing from psql command completion

Versions tested: 9.4.4, 9.5a2

Steps to reproduce:

1. psql to a database with sequences

2. type "grant usage on " and hit tab

3. "sequence" does not appear

4. type "grant usage on sequence " and hit tab

5. completes with word "to" which is incorrect.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Thu, Aug 20, 2015 at 2:54 PM, Josh Berkus <josh@agliodbs.com> wrote:
> Versions tested: 9.4.4, 9.5a2
>
> Steps to reproduce:
>
> 1. psql to a database with sequences
>
> 2. type "grant usage on " and hit tab
>
> 3. "sequence" does not appear
>
> 4. type "grant usage on sequence " and hit tab
>
> 5. completes with word "to" which is incorrect.

Here's a patch for that.

--
Thomas Munro
http://www.enterprisedb.com

On 08/19/2015 08:10 PM, Thomas Munro wrote:
> On Thu, Aug 20, 2015 at 2:54 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> Versions tested: 9.4.4, 9.5a2
>>
>> Steps to reproduce:
>>
>> 1. psql to a database with sequences
>>
>> 2. type "grant usage on " and hit tab
>>
>> 3. "sequence" does not appear
>>
>> 4. type "grant usage on sequence " and hit tab
>>
>> 5. completes with word "to" which is incorrect.
>
> Here's a patch for that.
>

Passes my ad-hoc tests, including non-default schema.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Tue, Sep 1, 2015 at 11:42 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Thu, Aug 20, 2015 at 3:20 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Thu, Aug 20, 2015 at 12:10 PM, Thomas Munro
>> <thomas.munro@enterprisedb.com> wrote:
>>>
>>> On Thu, Aug 20, 2015 at 2:54 PM, Josh Berkus <josh@agliodbs.com> wrote:
>>> > Versions tested: 9.4.4, 9.5a2
>>> >
>>> > Steps to reproduce:
>>> >
>>> > 1. psql to a database with sequences
>>> >
>>> > 2. type "grant usage on " and hit tab
>>> >
>>> > 3. "sequence" does not appear
>>> >
>>> > 4. type "grant usage on sequence " and hit tab
>>> >
>>> > 5. completes with word "to" which is incorrect.
>>>
>>> Here's a patch for that.
>>
>>
>> Don't we want to add TABLE as well? That's the default with just ON but it
>> seems useful to me to show up only a list of tables without all those
>> keywords.
>
>
> Agreed.  I noticed a couple more things:
>
> 1.  GRANT/REVOKE * ON DATABASE/SEQUENCE/TABLE/... * TO/FROM <tab> doesn't
> suggest roles, as it should.
> 2.  GRANT/REVOKE * ON ALL FUNCTIONS/SEQUENCES/TABLES IN SCHEMA ... isn't
> supported and might as well be.

Agreed on both points.

> New patch attached to address those.  I added this to the open commitfest.

Those are incorrect suggestions:
=# grant ALL  on ALL sequences in schema public from
postgres PUBLIC
=# revoke ALL  on ALL sequences in schema public to
postgres PUBLIC

And that's caused by this monster:
+       else if (((pg_strcasecmp(prev9_wd, "GRANT") == 0 ||
pg_strcasecmp(prev9_wd, "REVOKE") == 0) &&
+                         pg_strcasecmp(prev7_wd, "ON") == 0 &&
+                         pg_strcasecmp(prev6_wd, "ALL") == 0 &&
+                         pg_strcasecmp(prev4_wd, "IN") == 0 &&
+                         pg_strcasecmp(prev3_wd, "SCHEMA") == 0 &&
+                         (pg_strcasecmp(prev_wd, "TO") == 0 ||
pg_strcasecmp(prev_wd, "FROM") == 0)) ||
+                        ((pg_strcasecmp(prev6_wd, "GRANT") == 0 ||
pg_strcasecmp(prev6_wd, "REVOKE") == 0) &&
+                         pg_strcasecmp(prev4_wd, "ON") == 0 &&
+                         (pg_strcasecmp(prev_wd, "TO") == 0 ||
pg_strcasecmp(prev_wd, "FROM") == 0)) ||
+                        ((pg_strcasecmp(prev5_wd, "GRANT") == 0 ||
pg_strcasecmp(prev5_wd, "REVOKE") == 0) &&
+                         pg_strcasecmp(prev3_wd, "ON") == 0 &&
+                         (pg_strcasecmp(prev_wd, "TO") == 0 ||
pg_strcasecmp(prev_wd, "FROM") == 0)))
Could it be possible to simplify it a bit? You could either separate
it in two for REVOKE and GRANT or use an inner evaluation after
SCHEMA.
Regards,
--
Michael

On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
> Here is a version that splits that monster up into three small smaller
> blocks, and makes sure that GRANT goes with TO and REVOKE goes with FROM
> before completing with roles.
>
> Unfortunately your first example "GRANT ... FROM <tab>" still gets
> inappropriate completion because of the general FROM-matching branch with
> comment /* ... FROM ... */ that comes near the end, but it didn't seem
> sensible to start teaching the general FROM branch about avoiding this
> specific invalid production when it's happy to complete "BANANA FROM <tab>".

OK, let's live with that, tab completion would just have an incorrect
suggestion only once "from" is written completely with a space added
after it. Your patch improves many areas anyway, and that's just a
small point, hence let's have a committer look at it.
Regards,
--
Michael

On Wed, Sep 2, 2015 at 10:14 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
>> Here is a version that splits that monster up into three small smaller
>> blocks, and makes sure that GRANT goes with TO and REVOKE goes with FROM
>> before completing with roles.
>>
>> Unfortunately your first example "GRANT ... FROM <tab>" still gets
>> inappropriate completion because of the general FROM-matching branch with
>> comment /* ... FROM ... */ that comes near the end, but it didn't seem
>> sensible to start teaching the general FROM branch about avoiding this
>> specific invalid production when it's happy to complete "BANANA FROM <tab>".
>
> OK, let's live with that, tab completion would just have an incorrect
> suggestion only once "from" is written completely with a space added
> after it. Your patch improves many areas anyway, and that's just a
> small point, hence let's have a committer look at it.

"GRANT xxx ON FOREIGN DATA WRAPPER yyy <tab>" should suggest "TO"?
"GRANT xxx ON FOREIGN DATA WRAPPER yyy TO <tab>" should suggest the roles?
"GRANT xxx ON FOREIGN SERVER <tab>" should suggest foreign servers?
"GRANT xxx ON FOREIGN SERVER yyy <tab>" should suggest "TO"?
"GRANT xxx ON FOREIGN SERVER yyy TO <tab>" should suggest the roles?

Regards,

--
Fujii Masao

On Fri, Sep 4, 2015 at 7:24 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Fri, Sep 4, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> On Wed, Sep 2, 2015 at 10:14 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
>> >> Here is a version that splits that monster up into three small smaller
>> >> blocks, and makes sure that GRANT goes with TO and REVOKE goes with
>> >> FROM
>> >> before completing with roles.
>> >>
>> >> Unfortunately your first example "GRANT ... FROM <tab>" still gets
>> >> inappropriate completion because of the general FROM-matching branch
>> >> with
>> >> comment /* ... FROM ... */ that comes near the end, but it didn't seem
>> >> sensible to start teaching the general FROM branch about avoiding this
>> >> specific invalid production when it's happy to complete "BANANA FROM
>> >> <tab>".
>> >
>> > OK, let's live with that, tab completion would just have an incorrect
>> > suggestion only once "from" is written completely with a space added
>> > after it. Your patch improves many areas anyway, and that's just a
>> > small point, hence let's have a committer look at it.
>>
>> "GRANT xxx ON FOREIGN DATA WRAPPER yyy <tab>" should suggest "TO"?
>> "GRANT xxx ON FOREIGN DATA WRAPPER yyy TO <tab>" should suggest the roles?
>> "GRANT xxx ON FOREIGN SERVER <tab>" should suggest foreign servers?
>> "GRANT xxx ON FOREIGN SERVER yyy <tab>" should suggest "TO"?
>> "GRANT xxx ON FOREIGN SERVER yyy TO <tab>" should suggest the roles?
>
>
> Thanks.  New version attached that handles these to.

Thanks for updating the patch!
Attached is the updated version of the patch. Could you review this?

> Also fixed "GRANT * ON FOREIGN DATA <tab>"  which now suggests "WRAPPER".

Isn't this overkill? Otherwise, for example, "GRANT * ON ALL FUNCTIONS <tab>"
should suggest "IN", and then "GRANT * ON ALL FUNCTIONS IN <tab>" should
suggest "SCHEMA" for the sake of consistency. They seem overkill to me.
So I removed the code related to this from the patch.

+        else if (pg_strcasecmp(prev_wd, "TABLE") == 0)
+            COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);

Not only ordinary table but also ordinary view, materialized view,
foreign table, and sequence can follow the keyword TABLE. So I modified
the patch so that Query_for_list_of_tsvmf is used here, instead.

+    /*
+     * Complete "GRANT/REVOKE * ON ALL * IN SCHEMA * TO/FROM" with username,
+     * GROUP, or PUBLIC.
+     */

In 9.5 or later, CURRENT_USER or SESSION_USER keyword can follow TO/FROM.
So I added them into Query_for_list_of_grant_roles, and changed the above
comment.

+    else if (((pg_strcasecmp(prev9_wd, "GRANT") == 0 &&
pg_strcasecmp(prev_wd, "TO") == 0) ||
+              (pg_strcasecmp(prev9_wd, "REVOKE") == 0 &&
pg_strcasecmp(prev_wd, "FROM") == 0)) &&
+             pg_strcasecmp(prev7_wd, "ON") == 0 &&
+             pg_strcasecmp(prev6_wd, "ALL") == 0 &&
+             pg_strcasecmp(prev4_wd, "IN") == 0 &&
+             pg_strcasecmp(prev3_wd, "SCHEMA") == 0)
+        COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles);

Do we really need to check the keywords other than GRANT, REVOKE, TO and FROM?
You added several similar tab-completion codes like that, but I think that
we can refactor them so that only GRANT, REVOKE, TO and FROM are checked.
I applied that refactoring to the patch.

Regards,

--
Fujii Masao

On Fri, Sep 4, 2015 at 8:16 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

> On Fri, Sep 4, 2015 at 7:24 AM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
> > On Fri, Sep 4, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com>
> wrote:
> >>
> >> On Wed, Sep 2, 2015 at 10:14 AM, Michael Paquier
> >> <michael.paquier@gmail.com> wrote:
> >> > On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
> >> >> Here is a version that splits that monster up into three small
> smaller
> >> >> blocks, and makes sure that GRANT goes with TO and REVOKE goes with
> >> >> FROM
> >> >> before completing with roles.
> >> >>
> >> >> Unfortunately your first example "GRANT ... FROM <tab>" still gets
> >> >> inappropriate completion because of the general FROM-matching branch
> >> >> with
> >> >> comment /* ... FROM ... */ that comes near the end, but it didn't
> seem
> >> >> sensible to start teaching the general FROM branch about avoiding
> this
> >> >> specific invalid production when it's happy to complete "BANANA FROM
> >> >> <tab>".
> >> >
> >> > OK, let's live with that, tab completion would just have an incorrect
> >> > suggestion only once "from" is written completely with a space added
> >> > after it. Your patch improves many areas anyway, and that's just a
> >> > small point, hence let's have a committer look at it.
> >>
> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy <tab>" should suggest "TO"?
> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy TO <tab>" should suggest the
> roles?
> >> "GRANT xxx ON FOREIGN SERVER <tab>" should suggest foreign servers?
> >> "GRANT xxx ON FOREIGN SERVER yyy <tab>" should suggest "TO"?
> >> "GRANT xxx ON FOREIGN SERVER yyy TO <tab>" should suggest the roles?
> >
> >
> > Thanks.  New version attached that handles these to.
>
> Thanks for updating the patch!
> Attached is the updated version of the patch. Could you review this?


Thanks for the review and the improvements!


> > Also fixed "GRANT * ON FOREIGN DATA <tab>"  which now suggests "WRAPPER".
>
> Isn't this overkill? Otherwise, for example, "GRANT * ON ALL FUNCTIONS
> <tab>"
> should suggest "IN", and then "GRANT * ON ALL FUNCTIONS IN <tab>" should
> suggest "SCHEMA" for the sake of consistency. They seem overkill to me.
> So I removed the code related to this from the patch.
>

Fair enough.


> +        else if (pg_strcasecmp(prev_wd, "TABLE") == 0)
> +            COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
>
> Not only ordinary table but also ordinary view, materialized view,
> foreign table, and sequence can follow the keyword TABLE. So I modified
> the patch so that Query_for_list_of_tsvmf is used here, instead.
>
> +    /*
> +     * Complete "GRANT/REVOKE * ON ALL * IN SCHEMA * TO/FROM" with
> username,
> +     * GROUP, or PUBLIC.
> +     */
>
> In 9.5 or later, CURRENT_USER or SESSION_USER keyword can follow TO/FROM.
> So I added them into Query_for_list_of_grant_roles, and changed the above
> comment.
>

Good catch.


> +    else if (((pg_strcasecmp(prev9_wd, "GRANT") == 0 &&
> pg_strcasecmp(prev_wd, "TO") == 0) ||
> +              (pg_strcasecmp(prev9_wd, "REVOKE") == 0 &&
> pg_strcasecmp(prev_wd, "FROM") == 0)) &&
> +             pg_strcasecmp(prev7_wd, "ON") == 0 &&
> +             pg_strcasecmp(prev6_wd, "ALL") == 0 &&
> +             pg_strcasecmp(prev4_wd, "IN") == 0 &&
> +             pg_strcasecmp(prev3_wd, "SCHEMA") == 0)
> +        COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles);
>
> Do we really need to check the keywords other than GRANT, REVOKE, TO and
> FROM?
> You added several similar tab-completion codes like that, but I think that
> we can refactor them so that only GRANT, REVOKE, TO and FROM are checked.
> I applied that refactoring to the patch.
>

+1

--
Thomas Munro
http://www.enterprisedb.com

On Sat, Sep 5, 2015 at 9:43 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Fri, Sep 4, 2015 at 8:16 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> On Fri, Sep 4, 2015 at 7:24 AM, Thomas Munro
>> <thomas.munro@enterprisedb.com> wrote:
>> > On Fri, Sep 4, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com>
>> > wrote:
>> >>
>> >> On Wed, Sep 2, 2015 at 10:14 AM, Michael Paquier
>> >> <michael.paquier@gmail.com> wrote:
>> >> > On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
>> >> >> Here is a version that splits that monster up into three small
>> >> >> smaller
>> >> >> blocks, and makes sure that GRANT goes with TO and REVOKE goes with
>> >> >> FROM
>> >> >> before completing with roles.
>> >> >>
>> >> >> Unfortunately your first example "GRANT ... FROM <tab>" still gets
>> >> >> inappropriate completion because of the general FROM-matching branch
>> >> >> with
>> >> >> comment /* ... FROM ... */ that comes near the end, but it didn't
>> >> >> seem
>> >> >> sensible to start teaching the general FROM branch about avoiding
>> >> >> this
>> >> >> specific invalid production when it's happy to complete "BANANA FROM
>> >> >> <tab>".
>> >> >
>> >> > OK, let's live with that, tab completion would just have an incorrect
>> >> > suggestion only once "from" is written completely with a space added
>> >> > after it. Your patch improves many areas anyway, and that's just a
>> >> > small point, hence let's have a committer look at it.
>> >>
>> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy <tab>" should suggest "TO"?
>> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy TO <tab>" should suggest the
>> >> roles?
>> >> "GRANT xxx ON FOREIGN SERVER <tab>" should suggest foreign servers?
>> >> "GRANT xxx ON FOREIGN SERVER yyy <tab>" should suggest "TO"?
>> >> "GRANT xxx ON FOREIGN SERVER yyy TO <tab>" should suggest the roles?
>> >
>> >
>> > Thanks.  New version attached that handles these to.
>>
>> Thanks for updating the patch!
>> Attached is the updated version of the patch. Could you review this?
>
>
> Thanks for the review and the improvements!

So applied. Thanks a lot!

Regards,

--
Fujii Masao

On Tue, Sep 8, 2015 at 10:02 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

> On Sat, Sep 5, 2015 at 9:43 PM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
> > On Fri, Sep 4, 2015 at 8:16 PM, Fujii Masao <masao.fujii@gmail.com>
> wrote:
> >>
> >> On Fri, Sep 4, 2015 at 7:24 AM, Thomas Munro
> >> <thomas.munro@enterprisedb.com> wrote:
> >> > On Fri, Sep 4, 2015 at 12:02 AM, Fujii Masao <masao.fujii@gmail.com>
> >> > wrote:
> >> >>
> >> >> On Wed, Sep 2, 2015 at 10:14 AM, Michael Paquier
> >> >> <michael.paquier@gmail.com> wrote:
> >> >> > On Tue, Sep 1, 2015 at 10:15 PM, Thomas Munro wrote:
> >> >> >> Here is a version that splits that monster up into three small
> >> >> >> smaller
> >> >> >> blocks, and makes sure that GRANT goes with TO and REVOKE goes
> with
> >> >> >> FROM
> >> >> >> before completing with roles.
> >> >> >>
> >> >> >> Unfortunately your first example "GRANT ... FROM <tab>" still gets
> >> >> >> inappropriate completion because of the general FROM-matching
> branch
> >> >> >> with
> >> >> >> comment /* ... FROM ... */ that comes near the end, but it didn't
> >> >> >> seem
> >> >> >> sensible to start teaching the general FROM branch about avoiding
> >> >> >> this
> >> >> >> specific invalid production when it's happy to complete "BANANA
> FROM
> >> >> >> <tab>".
> >> >> >
> >> >> > OK, let's live with that, tab completion would just have an
> incorrect
> >> >> > suggestion only once "from" is written completely with a space
> added
> >> >> > after it. Your patch improves many areas anyway, and that's just a
> >> >> > small point, hence let's have a committer look at it.
> >> >>
> >> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy <tab>" should suggest "TO"?
> >> >> "GRANT xxx ON FOREIGN DATA WRAPPER yyy TO <tab>" should suggest the
> >> >> roles?
> >> >> "GRANT xxx ON FOREIGN SERVER <tab>" should suggest foreign servers?
> >> >> "GRANT xxx ON FOREIGN SERVER yyy <tab>" should suggest "TO"?
> >> >> "GRANT xxx ON FOREIGN SERVER yyy TO <tab>" should suggest the roles?
> >> >
> >> >
> >> > Thanks.  New version attached that handles these to.
> >>
> >> Thanks for updating the patch!
> >> Attached is the updated version of the patch. Could you review this?
> >
> >
> > Thanks for the review and the improvements!
>
> So applied. Thanks a lot!
>

Since this commit, "grant update on foobar to " will tab complete to add an
extra "TO", rather than with a list of roles.

Cheers,

Jeff

On Wed, Sep 30, 2015 at 5:59 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
> Since this commit, "grant update on foobar to " will tab complete to add an
> extra "TO", rather than with a list of roles.

Oops, thanks.  Here is a patch to fix that by moving the branch that
matches "GRANT/REVOKE * ON * *" after the branch that matches
"GRANT/REVOKE ... TO/FROM".

--
Thomas Munro
http://www.enterprisedb.com

On Tue, Sep 29, 2015 at 12:59 PM, Thomas Munro <
thomas.munro@enterprisedb.com> wrote:

> On Wed, Sep 30, 2015 at 5:59 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
> > Since this commit, "grant update on foobar to " will tab complete to add
> an
> > extra "TO", rather than with a list of roles.
>
> Oops, thanks.  Here is a patch to fix that by moving the branch that
> matches "GRANT/REVOKE * ON * *" after the branch that matches
> "GRANT/REVOKE ... TO/FROM".
>
>
Looks good to me, thanks.

Jeff

On Thu, Oct 1, 2015 at 2:03 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Tue, Sep 29, 2015 at 12:59 PM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>>
>> On Wed, Sep 30, 2015 at 5:59 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>> > Since this commit, "grant update on foobar to " will tab complete to add
>> > an
>> > extra "TO", rather than with a list of roles.
>>
>> Oops, thanks.  Here is a patch to fix that by moving the branch that
>> matches "GRANT/REVOKE * ON * *" after the branch that matches
>> "GRANT/REVOKE ... TO/FROM".
>>
>
> Looks good to me, thanks.

Applied. Thanks!

Regards,

--
Fujii Masao

On 10/01/2015 07:40 AM, Fujii Masao wrote:
> On Thu, Oct 1, 2015 at 2:03 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>> On Tue, Sep 29, 2015 at 12:59 PM, Thomas Munro
>> <thomas.munro@enterprisedb.com> wrote:
>>>
>>> On Wed, Sep 30, 2015 at 5:59 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>>>> Since this commit, "grant update on foobar to " will tab complete to add
>>>> an
>>>> extra "TO", rather than with a list of roles.
>>>
>>> Oops, thanks.  Here is a patch to fix that by moving the branch that
>>> matches "GRANT/REVOKE * ON * *" after the branch that matches
>>> "GRANT/REVOKE ... TO/FROM".
>>>
>>
>> Looks good to me, thanks.
>
> Applied. Thanks!

Wow, I had no idea this was going to be such a difficult fix ...


--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Oct 2, 2015 at 7:48 AM, Josh Berkus <josh@agliodbs.com> wrote:
> On 10/01/2015 07:40 AM, Fujii Masao wrote:
>> On Thu, Oct 1, 2015 at 2:03 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>>> On Tue, Sep 29, 2015 at 12:59 PM, Thomas Munro
>>> <thomas.munro@enterprisedb.com> wrote:
>>>>
>>>> On Wed, Sep 30, 2015 at 5:59 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
>>>>> Since this commit, "grant update on foobar to " will tab complete to add
>>>>> an
>>>>> extra "TO", rather than with a list of roles.
>>>>
>>>> Oops, thanks.  Here is a patch to fix that by moving the branch that
>>>> matches "GRANT/REVOKE * ON * *" after the branch that matches
>>>> "GRANT/REVOKE ... TO/FROM".
>>>>
>>>
>>> Looks good to me, thanks.
>>
>> Applied. Thanks!
>
> Wow, I had no idea this was going to be such a difficult fix ...

Well, a 3 line patch for the specific problem you reported was posted
straight away.

The rest of this thread got carried away filling out a bunch of other
GRANT/REVOKE completions!  And there are probably more things to do
(for example WITH GRANT OPTION).

FWIW I am hoping to make it a bit less cumbersome to maintain the
completion code in the next CF:
https://commitfest.postgresql.org/7/375/

--
Thomas Munro
http://www.enterprisedb.com