Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE - Mailing list pgsql-hackers
From | vignesh C |
---|---|
Subject | Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE |
Date | |
Msg-id | CALDaNm0VdEdCoYQ_+8_pSkzU+k7aDHC5s+d86HSXSd4Pmsk3uQ@mail.gmail.com Whole thread Raw |
In response to | Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE (Masahiko Sawada <sawada.mshk@gmail.com>) |
Responses |
Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE
|
List | pgsql-hackers |
On Tue, 2 Apr 2024 at 13:08, Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Mon, Apr 1, 2024 at 10:41 PM vignesh C <vignesh21@gmail.com> wrote: > > > > On Thu, 28 Mar 2024 at 13:05, Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > > > Hi, > > > > > > Thank you for the patch! > > > > > > On Mon, Jul 3, 2023 at 12:12 AM vignesh C <vignesh21@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
Attachment
pgsql-hackers by date: