On Thu, 28 Mar 2024 at 13:05, Masahiko Sawada <[email protected]> wrote:
>
> Hi,
>
> Thank you for the patch!
>
> On Mon, Jul 3, 2023 at 12:12 AM vignesh C <[email protected]> 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 <[email protected]>
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
