Re: Fix some newly modified tab-complete changes - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Fix some newly modified tab-complete changes
Date
Msg-id 20220928.144924.523902499458249904.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Fix some newly modified tab-complete changes  (Peter Smith <smithpb2250@gmail.com>)
Responses RE: Fix some newly modified tab-complete changes
List pgsql-hackers
At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith <smithpb2250@gmail.com> wrote in 
> On Tue, Sep 27, 2022 at 8:28 PM shiy.fnst@fujitsu.com
> <shiy.fnst@fujitsu.com> wrote:
> >
> > Hi hackers,
> >
> > I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should
> > be "ALL TABLES IN SCHEMA" in the following case.
> >
> > postgres=# grant all on
> > ALL FUNCTIONS IN SCHEMA   DATABASE                  FUNCTION                  PARAMETER                 SCHEMA
             TABLESPACE
 
> > ALL PROCEDURES IN SCHEMA  DOMAIN                    information_schema.       PROCEDURE                 SEQUENCE
             tbl
 
> > ALL ROUTINES IN SCHEMA    FOREIGN DATA WRAPPER      LANGUAGE                  public.                   TABLE
             TYPE
 
> > ALL SEQUENCES IN SCHEMA   FOREIGN SERVER            LARGE OBJECT              ROUTINE                   TABLES IN
SCHEMA
> >
> > I found that it is related to the recent commit 790bf615dd, and maybe it's
> > better to fix it. I also noticed that some comments should be modified according
> > to this new syntax. Attach a patch to fix them.
> >
> 
> Thanks for the patch! Below are my review comments.
> 
> The patch looks good to me but I did find some other tab-completion
> anomalies. IIUC these are unrelated to your work, but since I found
> them while testing your patch I am reporting them here.

Looks fine to me, too.

> Perhaps you want to fix them in the same patch, or just raise them
> again separately?
> 
> ======
> 
> 1. tab complete for CREATE PUBLICATION
> 
> I don’t think this is any new bug, but I found that it is possible to do this...
> 
> test_pub=# create publication p for ALL TABLES IN SCHEMA <tab>
> information_schema  pg_catalog          pg_toast            public
>
> or, even this...
> 
> test_pub=# create publication p for XXX TABLES IN SCHEMA <tab>
> information_schema  pg_catalog          pg_toast            public

Completion is responding to "IN SCHEMA" in these cases.  However, I
don't reach this state only by completion becuase it doesn't suggest
"IN SCHEMA" after "TABLES" nor "ALL TABLES".  I don't see a reason to
change that behavior unless that fix doesn't cause any additional
complexity.

> ======
> 
> 2. tab complete for GRANT
> 
> test_pub=# grant <tab>
> ALL                        EXECUTE
> pg_execute_server_program  pg_read_server_files       postgres
>           TRIGGER
> ALTER SYSTEM               GRANT                      pg_monitor
>           pg_signal_backend          REFERENCES
> TRUNCATE
> CONNECT                    INSERT                     pg_read_all_data
>           pg_stat_scan_tables        SELECT                     UPDATE
> CREATE                     pg_checkpoint
> pg_read_all_settings       pg_write_all_data          SET
>           USAGE
> DELETE                     pg_database_owner
> pg_read_all_stats          pg_write_server_files      TEMPORARY
> 
> 2a.
> grant "GRANT" ??

Yeah, for the mement I thought that might a kind of admin option but
there's no such a privilege. REVOKE gets the same suggestion.

> 2b.
> grant "TEMPORARY" but not "TEMP" ??

TEMP is an alternative spelling so that's fine.


I found the following suggestion.

CREATE PUBLICATION p FOR TABLES <tab> -> ["IN SCHEMA", "WITH ("]

I believe "WITH (" doesn't come there.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: longfin and tamandua aren't too happy but I'm not sure why
Next
From: Michael Paquier
Date:
Subject: Re: [PATCH] Add peer authentication TAP test