On Wed, Sep 28, 2022 1:49 PM Kyotaro Horiguchi <[email protected]> wrote: > > At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith > <[email protected]> wrote in > > On Tue, Sep 27, 2022 at 8:28 PM [email protected] > > <[email protected]> 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. >
Thanks for reviewing it.
> > 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.
>
+1
> > ======
> >
> > 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.
>
Maybe that's for "REVOKE GRANT OPTION FOR". But it is used by both GRANT and
REVOKE. I think it's a separate problem, I have tried to fix it in the attached
0002 patch.
> > 2b.
> > grant "TEMPORARY" but not "TEMP" ??
>
> TEMP is an alternative spelling so that's fine.
>
Agreed.
>
> I found the following suggestion.
>
> CREATE PUBLICATION p FOR TABLES <tab> -> ["IN SCHEMA", "WITH ("]
>
> I believe "WITH (" doesn't come there.
>
Fixed.
Attach the updated patch.
Regards,
Shi yu
v2-0001-Fix-some-newly-modified-tab-complete-changes-and-.patch
Description: v2-0001-Fix-some-newly-modified-tab-complete-changes-and-.patch
v2-0002-Fix-tab-completion-for-GRANT-REVOKE.patch
Description: v2-0002-Fix-tab-completion-for-GRANT-REVOKE.patch
