Re: GRANT USAGE ON SEQUENCE missing from psql command completion

On Tue, Sep 1, 2015 at 4:24 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Tue, Sep 1, 2015 at 11:42 AM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> > On Thu, Aug 20, 2015 at 3:20 PM, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com>
> > wrote:
> >>
> >> On Thu, Aug 20, 2015 at 12:10 PM, Thomas Munro
> >> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> >>>
> >>> On Thu, Aug 20, 2015 at 2:54 PM, Josh Berkus <josh(at)agliodbs(dot)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>".

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