On Tue, 2 Apr 2024 at 13:08, Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Mon, Apr 1, 2024 at 10:41 PM vignesh C <vignes...@gmail.com> wrote: > > > > On Thu, 28 Mar 2024 at 13:05, Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > > > Hi, > > > > > > Thank you for the patch! > > > > > > On Mon, Jul 3, 2023 at 12:12 AM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > Hi, > > > > > > > > Improved tab completion for "ALTER DEFAULT PRIVILEGE" and "ALTER TABLE": > > > > 1) GRANT, REVOKE and FOR USER keyword was not displayed in tab > > > > completion of alter default privileges like the below statement: > > > > ALTER DEFAULT PRIVILEGES GRANT INSERT ON tables TO PUBLIC; > > > > ALTER DEFAULT PRIVILEGES REVOKE INSERT ON tables FROM PUBLIC; > > > > ALTER DEFAULT PRIVILEGES FOR USER vignesh revoke INSERT ON tables FROM > > > > dba1; > > > > > > +1 > > > > > > > > > > > 2) USER was not displayed for "ALTER DEFAULT PRIVILEGES IN SCHEMA > > > > public FOR " like in below statement: > > > > ALTER DEFAULT PRIVILEGES IN SCHEMA public FOR USER dba1 GRANT INSERT > > > > ON TABLES TO PUBLIC; > > > > > > Since there is no difference FOR USER and FOR ROLE, I'm not sure we > > > really want to support both in tab-completion. > > > > I have removed this change > > > > > > > > > > 3) "FOR GRANT OPTION" was not display for "ALTER DEFAULT PRIVILEGES > > > > REVOKE " like in below statement: > > > > alter default privileges revoke grant option for select ON tables FROM > > > > dba1; > > > > > > +1. But the v3 patch doesn't cover the following case: > > > > > > =# alter default privileges for role masahiko revoke [tab] > > > ALL CREATE DELETE EXECUTE INSERT MAINTAIN > > > REFERENCES SELECT TRIGGER TRUNCATE UPDATE USAGE > > > > Modified in the updated patch > > > > > And it doesn't cover MAINTAIN neither: > > > > > > =# alter default privileges revoke [tab] > > > ALL DELETE GRANT OPTION FOR REFERENCES > > > TRIGGER UPDATE > > > CREATE EXECUTE INSERT SELECT > > > TRUNCATE USAGE > > > > Modified in the updated patch > > > > > The patch adds the completions for ALTER DEFAULT PRIVILEGES REVOKE, > > > but we handle such case in GRANT and REVOKE part: > > > > > > (around L3958) > > > /* > > > * With ALTER DEFAULT PRIVILEGES, restrict completion to grantable > > > * privileges (can't grant roles) > > > */ > > > if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES")) > > > COMPLETE_WITH("SELECT", "INSERT", "UPDATE", > > > "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", > > > "CREATE", "EXECUTE", "USAGE", "MAINTAIN", > > > "ALL"); > > > > The current patch handles the fix from here now. > > > > > Also, I think we can support WITH GRANT OPTION too. For example, > > > > > > =# alter default privileges for role masahiko grant all on tables to > > > public [tab] > > > > I have handled this in the updated patch > > > > > It's already supported in the GRANT statement. > > > > > > > > > > > 4) "DATA TYPE" was missing in "ALTER TABLE table-name ALTER COLUMN > > > > column-name SET" like in: > > > > ALTER TABLE t1 ALTER COLUMN c1 SET DATA TYPE text; > > > > > > > > > > +1. The patch looks good to me, so pushed. > > > > Thanks for committing this. > > > > The updated patch has the changes for the above comments. > > > > Thank you for updating the patch. > > I think it doesn't work well as "GRANT OPTION FOR" is complemented > twice. For example, > > =# alter default privileges for user masahiko revoke [tab] > ALL DELETE GRANT OPTION FOR MAINTAIN > SELECT TRUNCATE USAGE > CREATE EXECUTE INSERT REFERENCES > TRIGGER UPDATE > =# alter default privileges for user masahiko revoke grant option for [tab] > ALL DELETE GRANT OPTION FOR MAINTAIN > SELECT TRUNCATE USAGE > CREATE EXECUTE INSERT REFERENCES > TRIGGER UPDATE
Thanks for finding this issue, the attached v5 version patch has the fix for the same. Regards, Vignesh
From 7aecc1d577f5608384044692dbffee7018e434fd Mon Sep 17 00:00:00 2001 From: Vignesh C <vignes...@gmail.com> Date: Sun, 2 Jul 2023 19:35:46 +0530 Subject: [PATCH v5] Fix missing tab completion in "ALTER DEFAULT PRIVILEGES" GRANT, REVOKE and FOR USER keyword was not displayed in tab completion of "ALTER DEFAULT PRIVILEGES" like the below statements: ALTER DEFAULT PRIVILEGES GRANT INSERT ON tables TO PUBLIC; ALTER DEFAULT PRIVILEGES REVOKE INSERT ON tables FROM PUBLIC; ALTER DEFAULT PRIVILEGES FOR USER testdba REVOKE INSERT ON tables FROM testdba; "GRANT OPTION OPTION" was not displayed in tab completion of "ALTER DEFAULT PRIVILEGES REVOKE " like in below statement: ALTER DEFAULT PRIVILEGES REVOKE GRANT OPTION FOR SELECT ON tables FROM testdba; "WITH GRANT OPTION" was not completed in tab completion of "ALTER DEFAULT PRIVILEGES ... GRANT ... to role" like in below statement: ALTER DEFAULT PRIVILEGES FOR ROLE testdba1 GRANT ALL ON tables to testdba2 WITH GRANT OPTION; --- src/bin/psql/tab-complete.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 82eb3955ab..0a9cdea9a2 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2147,7 +2147,7 @@ psql_completion(const char *text, int start, int end) /* ALTER DEFAULT PRIVILEGES */ else if (Matches("ALTER", "DEFAULT", "PRIVILEGES")) - COMPLETE_WITH("FOR ROLE", "IN SCHEMA"); + COMPLETE_WITH("FOR", "GRANT", "IN SCHEMA", "REVOKE"); /* ALTER DEFAULT PRIVILEGES FOR */ else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", "FOR")) COMPLETE_WITH("ROLE"); @@ -3949,9 +3949,18 @@ psql_completion(const char *text, int start, int end) * privileges (can't grant roles) */ if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES")) - COMPLETE_WITH("SELECT", "INSERT", "UPDATE", - "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", - "CREATE", "EXECUTE", "USAGE", "MAINTAIN", "ALL"); + { + if (TailMatches("GRANT") || + TailMatches("REVOKE", "GRANT", "OPTION", "FOR")) + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", + "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", + "CREATE", "EXECUTE", "USAGE", "MAINTAIN", "ALL"); + else if (TailMatches("REVOKE")) + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", + "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", + "CREATE", "EXECUTE", "USAGE", "MAINTAIN", + "GRANT OPTION FOR", "ALL"); + } else if (TailMatches("GRANT")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Privilege_options_of_grant_and_revoke); @@ -4133,6 +4142,10 @@ psql_completion(const char *text, int start, int end) else if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES") && TailMatches("TO|FROM")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, Keywords_for_list_of_grant_roles); + /* Complete "ALTER DEFAULT PRIVILEGES ... GRANT ... TO ROLE */ + else if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES") && + TailMatches("GRANT", MatchAny, MatchAny, MatchAny, "TO", MatchAny)) + COMPLETE_WITH("WITH GRANT OPTION"); /* Complete "GRANT/REVOKE ... ON * *" with TO/FROM */ else if (HeadMatches("GRANT") && TailMatches("ON", MatchAny, MatchAny)) COMPLETE_WITH("TO"); -- 2.34.1