Re: Improve tab completion for ALTER DEFAULT PRIVILEGE and ALTER TABLE

Hi,

Thank you for the patch!

On Mon, Jul 3, 2023 at 12:12 AM vignesh C <vignesh21(at)gmail(dot)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