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. Regards, Vignesh
From 7c3bbfb0c8c17bf5d4a7e2ce579d00b811844a06 Mon Sep 17 00:00:00 2001 From: Vignesh C <vignesh21@gmail.com> Date: Sun, 2 Jul 2023 19:35:46 +0530 Subject: [PATCH v4] 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 | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 82eb3955ab..620628948a 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,17 @@ 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")) + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", + "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", + "CREATE", "EXECUTE", "USAGE", "MAINTAIN", "ALL"); + else + 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 +4141,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