Обсуждение: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE
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; 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; 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; 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; Attached patch has the changes for the same. Regards, Vignesh
Вложения
On Sun, 2 Jul 2023 at 20:42, 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; > > 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; > > 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; > > 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; > > Attached patch has the changes for the same. Added a commitfest entry for this: https://commitfest.postgresql.org/45/4587/ Regards, Vignesh
n Fri, Nov 24, 2023 at 6:33 PM 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;
>
> 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;
>
> 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;
>
> 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;
>
> Attached patch has the changes for the same.
+ COMPLETE_WITH("ROLE", "USER");
+ /* ALTER DEFAULT PRIVILEGES REVOKE */
+ else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", "REVOKE"))
+ COMPLETE_WITH("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE",
+ "REFERENCES", "TRIGGER", "CREATE", "EXECUTE", "USAGE",
+ "MAINTAIN", "ALL", "GRANT OPTION FOR");
I could not find "alter default privileges revoke maintain", should
this be removed?
Regards,
Shubham Khanna
On Fri, 24 Nov 2023 at 18:37, Shubham Khanna
<khannashubham1197@gmail.com> wrote:
>
> n Fri, Nov 24, 2023 at 6:33 PM 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;
> >
> > 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;
> >
> > 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;
> >
> > 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;
> >
> > Attached patch has the changes for the same.
>
> + COMPLETE_WITH("ROLE", "USER");
> + /* ALTER DEFAULT PRIVILEGES REVOKE */
> + else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", "REVOKE"))
> + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE",
> + "REFERENCES", "TRIGGER", "CREATE", "EXECUTE", "USAGE",
> + "MAINTAIN", "ALL", "GRANT OPTION FOR");
>
> I could not find "alter default privileges revoke maintain", should
> this be removed?
This was reverted as part of:
151c22deee66a3390ca9a1c3675e29de54ae73fc.
Revert MAINTAIN privilege and pg_maintain predefined role.
This reverts the following commits: 4dbdb82513, c2122aae63,
5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d,
and b5d6382496. A role with the MAINTAIN privilege may be able to
use search_path tricks to escalate privileges to the table owner.
Unfortunately, it is too late in the v16 development cycle to apply
the proposed fix, i.e., restricting search_path when running
maintenance commands.
The attached v2 version has the changes for the same.
Regards,
Vignesh
Вложения
On Mon, Nov 27, 2023 at 9:58 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 24 Nov 2023 at 18:37, Shubham Khanna
> <khannashubham1197@gmail.com> wrote:
> >
> > n Fri, Nov 24, 2023 at 6:33 PM 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;
> > >
> > > 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;
> > >
> > > 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;
> > >
> > > 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;
> > >
> > > Attached patch has the changes for the same.
> >
> > + COMPLETE_WITH("ROLE", "USER");
> > + /* ALTER DEFAULT PRIVILEGES REVOKE */
> > + else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", "REVOKE"))
> > + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE",
> > + "REFERENCES", "TRIGGER", "CREATE", "EXECUTE", "USAGE",
> > + "MAINTAIN", "ALL", "GRANT OPTION FOR");
> >
> > I could not find "alter default privileges revoke maintain", should
> > this be removed?
>
> This was reverted as part of:
> 151c22deee66a3390ca9a1c3675e29de54ae73fc.
> Revert MAINTAIN privilege and pg_maintain predefined role.
>
> This reverts the following commits: 4dbdb82513, c2122aae63,
> 5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d,
> and b5d6382496. A role with the MAINTAIN privilege may be able to
> use search_path tricks to escalate privileges to the table owner.
> Unfortunately, it is too late in the v16 development cycle to apply
> the proposed fix, i.e., restricting search_path when running
> maintenance commands.
>
I have executed the given changes and they are working fine.
Thanks and Regards,
Shubham Khanna.
On Mon, 27 Nov 2023 at 21:58, vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 24 Nov 2023 at 18:37, Shubham Khanna
> <khannashubham1197@gmail.com> wrote:
> >
> > n Fri, Nov 24, 2023 at 6:33 PM 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;
> > >
> > > 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;
> > >
> > > 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;
> > >
> > > 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;
> > >
> > > Attached patch has the changes for the same.
> >
> > + COMPLETE_WITH("ROLE", "USER");
> > + /* ALTER DEFAULT PRIVILEGES REVOKE */
> > + else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", "REVOKE"))
> > + COMPLETE_WITH("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE",
> > + "REFERENCES", "TRIGGER", "CREATE", "EXECUTE", "USAGE",
> > + "MAINTAIN", "ALL", "GRANT OPTION FOR");
> >
> > I could not find "alter default privileges revoke maintain", should
> > this be removed?
>
> This was reverted as part of:
> 151c22deee66a3390ca9a1c3675e29de54ae73fc.
> Revert MAINTAIN privilege and pg_maintain predefined role.
>
> This reverts the following commits: 4dbdb82513, c2122aae63,
> 5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d,
> and b5d6382496. A role with the MAINTAIN privilege may be able to
> use search_path tricks to escalate privileges to the table owner.
> Unfortunately, it is too late in the v16 development cycle to apply
> the proposed fix, i.e., restricting search_path when running
> maintenance commands.
>
> The attached v2 version has the changes for the same.
The patch was not applying because of a recent commit in tab
completion, PSA new patch set.
Regards,
Vignesh
Вложения
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.
>
> 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
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
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");
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]
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.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
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.
Regards,
Vignesh
Вложения
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
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
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
Вложения
On Fri, Apr 5, 2024 at 1:18 AM vignesh C <vignesh21@gmail.com> wrote:
>
> 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.
Thank you for updating the patch! I've pushed with minor adjustments.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On Mon, 8 Apr 2024 at 10:29, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 1:18 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > 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.
>
> Thank you for updating the patch! I've pushed with minor adjustments.
Thanks for pushing this patch.
Regards,
Vignesh