Re: Fix some newly modified tab-complete changes

At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith <smithpb2250(at)gmail(dot)com> wrote in
> On Tue, Sep 27, 2022 at 8:28 PM shiy(dot)fnst(at)fujitsu(dot)com
> <shiy(dot)fnst(at)fujitsu(dot)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