Re: GRANT USAGE ON SEQUENCE missing from psql command completion - Mailing list pgsql-bugs

From Thomas Munro
Subject Re: GRANT USAGE ON SEQUENCE missing from psql command completion
Date
Msg-id CAEepm=163mu-qEpGNm2i78aYpVw+SmjCL=b87O9VcGcuppv9YA@mail.gmail.com
Whole thread Raw
In response to Re: GRANT USAGE ON SEQUENCE missing from psql command completion  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: GRANT USAGE ON SEQUENCE missing from psql command completion
List pgsql-bugs
On Tue, Sep 1, 2015 at 4:24 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
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.

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

--
Attachment

pgsql-bugs by date:

Previous
From: Michael Paquier
Date:
Subject: Re: GRANT USAGE ON SEQUENCE missing from psql command completion
Next
From: edicir.marmentini@gmail.com
Date:
Subject: BUG #13600: Impossível matar processo Postgres.exe