pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Tab completion for CREATE SCHEMAAUTHORIZATION
18+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2023-06-30 c951e9042d  Add tab completion for CREATE SCHEMA in psql
* Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-07-09 16:20  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: ilmari@ilmari.org @ 2021-07-09 16:20 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

Hi Hackers,

I just noticed there's no tab completion for CREATE SCHEMA
AUTHORIZATION, nor for anything after CREATE SCHEMA <name>.

Please find attached a patch that adds this.

- ilmari

Attachments:

0001-Add-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 2.1K) download From db02df7ea8d3a5db41268edd8c311a3631c9e9ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Fri, 9 Jul 2021 17:15:00 +0100 Subject: [PATCH] Add tab completion for CREATE SCHEMA --- src/bin/psql/tab-complete.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 0ebd5aa41a..80f6001073 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -758,6 +758,13 @@ static const SchemaQuery Query_for_list_of_collations = { " FROM pg_catalog.pg_roles "\ " WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'" +#define Query_for_list_of_schema_roles \ +" SELECT pg_catalog.quote_ident(rolname) "\ +" FROM pg_catalog.pg_roles "\ +" WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"\ +" UNION ALL SELECT 'CURRENT_USER'"\ +" UNION ALL SELECT 'SESSION_USER'" + #define Query_for_list_of_grant_roles \ " SELECT pg_catalog.quote_ident(rolname) "\ " FROM pg_catalog.pg_roles "\ @@ -2668,6 +2675,19 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY(Query_for_list_of_schemas + " UNION ALL SELECT 'AUTHORIZATION'"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_schema_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_schema_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) -- 2.30.2
^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-07-15 11:13  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: ilmari@ilmari.org @ 2021-07-15 11:13 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> Hi Hackers,
>
> I just noticed there's no tab completion for CREATE SCHEMA
> AUTHORIZATION, nor for anything after CREATE SCHEMA <name>.
>
> Please find attached a patch that adds this.

Added to the 2021-09 commit fest: https://commitfest.postgresql.org/34/3252/

- ilmari





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-08-07 21:09  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: ilmari@ilmari.org @ 2021-08-07 21:09 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
>
>> Hi Hackers,
>>
>> I just noticed there's no tab completion for CREATE SCHEMA
>> AUTHORIZATION, nor for anything after CREATE SCHEMA <name>.
>>
>> Please find attached a patch that adds this.
>
> Added to the 2021-09 commit fest: https://commitfest.postgresql.org/34/3252/

Here's an updated version that also reduces the duplication between the
various role list queries.

- ilmari

Attachments:

v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 2.5K) download From 52c1fe9900543253510b64e58de24f920d8b0dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Fri, 9 Jul 2021 17:15:00 +0100 Subject: [PATCH v2] Add tab completion for CREATE SCHEMA In passing, reduce duplication between the various role list queries --- src/bin/psql/tab-complete.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 064892bade..adddd45f59 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -758,15 +758,16 @@ static const SchemaQuery Query_for_list_of_collations = { " FROM pg_catalog.pg_roles "\ " WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'" -#define Query_for_list_of_grant_roles \ -" SELECT pg_catalog.quote_ident(rolname) "\ -" FROM pg_catalog.pg_roles "\ -" WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"\ -" UNION ALL SELECT 'PUBLIC'"\ -" UNION ALL SELECT 'CURRENT_ROLE'"\ +#define Query_for_list_of_schema_roles \ +Query_for_list_of_roles \ " UNION ALL SELECT 'CURRENT_USER'"\ " UNION ALL SELECT 'SESSION_USER'" +#define Query_for_list_of_grant_roles \ +Query_for_list_of_schema_roles \ +" UNION ALL SELECT 'PUBLIC'"\ +" UNION ALL SELECT 'CURRENT_ROLE'" + /* the silly-looking length condition is just to eat up the current word */ #define Query_for_index_of_table \ "SELECT pg_catalog.quote_ident(c2.relname) "\ @@ -2675,6 +2676,19 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY(Query_for_list_of_schemas + " UNION ALL SELECT 'AUTHORIZATION'"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_schema_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_schema_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) -- 2.30.2
^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-08-09 09:23  Suraj Khamkar <khamkarsuraj.b@gmail.com>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: Suraj Khamkar @ 2021-08-09 09:23 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers@lists.postgresql.org

Hello Dagfinn,

I had a look at your patch and below are my review comments.
Please correct me if I am missing something.

   1. For me the patch does not apply cleanly. I have been facing the error
   of trailing whitespaces.
   surajkhamkar@localhost:postgres$ git apply
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch:25: trailing
   whitespace.
   #define Query_for_list_of_schema_roles \
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch:26: trailing
   whitespace.
   Query_for_list_of_roles \
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch:30: trailing
   whitespace.
   #define Query_for_list_of_grant_roles \
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch:31: trailing
   whitespace.
   Query_for_list_of_schema_roles \
   v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch:32: trailing
   whitespace.
   " UNION ALL SELECT 'PUBLIC'"\
   error: patch failed: src/bin/psql/tab-complete.c:758
   error: src/bin/psql/tab-complete.c: patch does not apply

   2. We can remove space in before \ and below change
   +" UNION ALL SELECT 'PUBLIC'" \

   Should be,
   +" UNION ALL SELECT 'PUBLIC' "\

   3. role_specification has CURRENT_ROLE, CURRENT_USER and SESSION_USER.
   But current changes are missing CURRENT_ROLE.
   postgres@53724=#CREATE SCHEMA AUTHORIZATION
   CURRENT_USER               pg_execute_server_program  pg_read_all_data

   pg_read_all_stats          pg_signal_backend          pg_write_all_data

   SESSION_USER               pg_database_owner          pg_monitor

   pg_read_all_settings       pg_read_server_files
   pg_stat_scan_tables
   pg_write_server_files      surajkhamkar

   4. I'm not sure about this but do we need to enable tab completion for IF
   NOT EXIST?

   5. I think we are not handling IF NOT EXIST that's why it's not
   completing tab completion
   for AUTHORIZATION.

   6. As we are here we can also enable missing tab completion for ALTER
   SCHEMA.
   After OWNER TO we should also get CURRENT_ROLE, CURRENT_USER and
   SESSION_USER.
   postgres@53724=#ALTER SCHEMA sch owner to
   pg_database_owner          pg_monitor
   pg_read_all_settings
   pg_read_server_files       pg_stat_scan_tables
    pg_write_server_files
   pg_execute_server_program  pg_read_all_data           pg_read_all_stats

   pg_signal_backend          pg_write_all_data          surajkhamkar

   7. Similarly, as we can drop multiple schemas' simultaneously, we should
   enable tab completion for
   comma with CASCADE and RESTRICT
   postgres@53724=#DROP SCHEMA sch
   CASCADE   RESTRICT


Thanks.

On Sun, Aug 8, 2021 at 2:39 AM Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
wrote:

> ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
>
> > ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
> >
> >> Hi Hackers,
> >>
> >> I just noticed there's no tab completion for CREATE SCHEMA
> >> AUTHORIZATION, nor for anything after CREATE SCHEMA <name>.
> >>
> >> Please find attached a patch that adds this.
> >
> > Added to the 2021-09 commit fest:
> https://commitfest.postgresql.org/34/3252/
>
> Here's an updated version that also reduces the duplication between the
> various role list queries.
>
> - ilmari
>
>

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-08-09 18:00  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Suraj Khamkar <khamkarsuraj.b@gmail.com>
  0 siblings, 2 replies; 18+ messages in thread

From: ilmari@ilmari.org @ 2021-08-09 18:00 UTC (permalink / raw)
  To: Suraj Khamkar <khamkarsuraj.b@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org

Hi Suraj,

Suraj Khamkar <khamkarsuraj.b@gmail.com> writes:

> Hello Dagfinn,
>
> I had a look at your patch and below are my review comments.
> Please correct me if I am missing something.
>
>    1. For me the patch does not apply cleanly. I have been facing the error
>    of trailing whitespaces.

I do not get these errors, neither with the patch file I still have
locally, or by saving the attachment from my original email.  Are you
sure something in your download process hasn't converted it to Windows
line endings (\r\n), or otherwise mangled the whitespace?

>    2. We can remove space in before \ and below change
>    +" UNION ALL SELECT 'PUBLIC'" \
>
>    Should be,
>    +" UNION ALL SELECT 'PUBLIC' "\

The patch doesn't add any lines that end with quote-space-backslash.
As for the space before the quote, that is not necessary either, since the
next line starts with a space after the quote.  Either way, the updated
version of the patch doesn't add any new lines with continuation after a
string constant, so the point is moot.

>    3. role_specification has CURRENT_ROLE, CURRENT_USER and SESSION_USER.
>    But current changes are missing CURRENT_ROLE.

Ah, I was looking at the documentation for 13, but CURRENT_ROLE is only
allowed in this context as of 14.  Fixed.

>    4. I'm not sure about this but do we need to enable tab completion for IF
>    NOT EXIST?
>
>    5. I think we are not handling IF NOT EXIST that's why it's not
>    completing tab completion
>    for AUTHORIZATION.

As you note, psql currently doesn't currently tab-complete IF NOT EXISTS
for any command, so that would be a subject for a separate patch.

>    6. As we are here we can also enable missing tab completion for ALTER
>    SCHEMA.
>    After OWNER TO we should also get CURRENT_ROLE, CURRENT_USER and
>    SESSION_USER.

I did an audit of all the uses of Query_for_list_of_roles, and there
turned out be several more that accept CURRENT_ROLE, CURRENT_USER and
SESSION_USER that they weren't tab-completed for.  I also renamed the
constant to Query_for_list_of_owner_roles, but I'm not 100% happy with
that name either.

>    7. Similarly, as we can drop multiple schemas' simultaneously, we should
>    enable tab completion for
>    comma with CASCADE and RESTRICT
>    postgres@53724=#DROP SCHEMA sch
>    CASCADE   RESTRICT

The tab completion code for DROP is generic for all object types (see
the words_after_create array and the create_or_drop_command_generator
function), so that should be done genericallly, and is thus outside the
scope for this patch.

> Thanks.

Thanks for the review.  Updated patch attached, with the CURRENT/SESSION
ROLE/USER changes for other commands separated out.

- ilmari

Attachments: download all (2 files, .tar.gz)

v3-0001-Tab-complete-CURRENT_ROLE-CURRENT_USER-and-SESSIO.patch (text/x-diff, 4.3K) download From 68ebc42bf142f843dd3bf5741e085ec0397c8e0b Mon Sep 17 00:00:00 2001 From: tanghy <tanghy.fnst@fujitsu.com> Date: Mon, 9 Aug 2021 18:42:12 +0100 Subject: [PATCH v3 1/2] Tab-complete CURRENT_ROLE, CURRENT_USER and SESSION_USER everywhere All commands that manipulate ownership let you specify the current or session user by the above aliases, but the tab completion code had only got that memo in some places. --- src/bin/psql/tab-complete.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 064892bade..4ddc8134a0 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -758,15 +758,16 @@ static const SchemaQuery Query_for_list_of_collations = { " FROM pg_catalog.pg_roles "\ " WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'" -#define Query_for_list_of_grant_roles \ -" SELECT pg_catalog.quote_ident(rolname) "\ -" FROM pg_catalog.pg_roles "\ -" WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"\ -" UNION ALL SELECT 'PUBLIC'"\ +#define Query_for_list_of_owner_roles \ +Query_for_list_of_roles \ " UNION ALL SELECT 'CURRENT_ROLE'"\ " UNION ALL SELECT 'CURRENT_USER'"\ " UNION ALL SELECT 'SESSION_USER'" +#define Query_for_list_of_grant_roles \ +Query_for_list_of_owner_roles \ +" UNION ALL SELECT 'PUBLIC'" + /* the silly-looking length condition is just to eat up the current word */ #define Query_for_index_of_table \ "SELECT pg_catalog.quote_ident(c2.relname) "\ @@ -1613,7 +1614,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny, "OWNED", "BY")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny, "OWNED", "BY", MatchAny)) COMPLETE_WITH("SET TABLESPACE"); @@ -2257,7 +2258,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("USER"); /* complete ALTER GROUP <foo> ADD|DROP USER with a user name */ else if (Matches("ALTER", "GROUP", MatchAny, "ADD|DROP", "USER")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); /* * If we have ALTER TYPE <sth> RENAME VALUE, provide list of enum values @@ -3159,7 +3160,7 @@ psql_completion(const char *text, int start, int end) else if (Matches("DROP", "OWNED")) COMPLETE_WITH("BY"); else if (Matches("DROP", "OWNED", "BY")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); /* DROP TEXT SEARCH */ else if (Matches("DROP", "TEXT", "SEARCH")) @@ -3590,7 +3591,7 @@ psql_completion(const char *text, int start, int end) /* OWNER TO - complete with available roles */ else if (TailMatches("OWNER", "TO")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); /* ORDER BY */ else if (TailMatches("FROM", MatchAny, "ORDER")) @@ -3613,11 +3614,11 @@ psql_completion(const char *text, int start, int end) else if (Matches("REASSIGN", "OWNED")) COMPLETE_WITH("BY"); else if (Matches("REASSIGN", "OWNED", "BY")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); else if (Matches("REASSIGN", "OWNED", "BY", MatchAny)) COMPLETE_WITH("TO"); else if (Matches("REASSIGN", "OWNED", "BY", MatchAny, "TO")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); /* REFRESH MATERIALIZED VIEW */ else if (Matches("REFRESH")) @@ -3878,10 +3879,7 @@ psql_completion(const char *text, int start, int end) else if (Matches("ALTER|CREATE|DROP", "USER", "MAPPING")) COMPLETE_WITH("FOR"); else if (Matches("CREATE", "USER", "MAPPING", "FOR")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles - " UNION SELECT 'CURRENT_ROLE'" - " UNION SELECT 'CURRENT_USER'" - " UNION SELECT 'PUBLIC'" + COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles " UNION SELECT 'USER'"); else if (Matches("ALTER|DROP", "USER", "MAPPING", "FOR")) COMPLETE_WITH_QUERY(Query_for_list_of_user_mappings); -- 2.30.2
v3-0002-Add-more-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 1.7K) download From ecfc198c5daec4bad701e63f726810b059abf844 Mon Sep 17 00:00:00 2001 From: tanghy <tanghy.fnst@fujitsu.com> Date: Mon, 9 Aug 2021 18:47:12 +0100 Subject: [PATCH v3 2/2] Add more tab completion for CREATE SCHEMA - AUTHORIZATION both instead of and after a schema name - list of roles after AUTHORIZATION - CREATE and GRANT after any otherwise-complete command --- src/bin/psql/tab-complete.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 4ddc8134a0..20bcbedf92 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2676,6 +2676,19 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY(Query_for_list_of_schemas + " UNION ALL SELECT 'AUTHORIZATION'"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles); + else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) -- 2.30.2
^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-08-11 01:16  Michael Paquier <michael@paquier.xyz>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  1 sibling, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2021-08-11 01:16 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Mon, Aug 09, 2021 at 07:00:02PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Thanks for the review.  Updated patch attached, with the CURRENT/SESSION
> ROLE/USER changes for other commands separated out.

+#define Query_for_list_of_owner_roles \
+Query_for_list_of_roles \
 " UNION ALL SELECT 'CURRENT_ROLE'"\
 " UNION ALL SELECT 'CURRENT_USER'"\
 " UNION ALL SELECT 'SESSION_USER'"
I don't object to the refactoring you are doing here with three
Query_for_list_of_*_roles each one depending on the other for clarity.
Neither do I really object to not using COMPLETE_WITH_QUERY() with
some extra UNION ALL hardcoded in each code path as there 6 cases for
_owner_, 6 for _grant_ and 6 for _roles if my count is right.  Still,
if I may ask, wouldn't it be better to document a bit what's the
expectation behind each one of them?  Perhaps the names of the queries
are too generic for the purposes where they are used (say _grant_ for
CREATE USER MAPPING looks confusing)?

+       else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION"))
+               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
+       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION"))
+               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
+       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
+               COMPLETE_WITH("CREATE", "GRANT");
+       else if (Matches("CREATE", "SCHEMA", MatchAny))
+               COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
Looks like you forgot the case "CREATE SCHEMA AUTHORIZATION MatchAny"
that should be completed by GRANT and CREATE.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-08-11 17:34  Suraj Khamkar <khamkarsuraj.b@gmail.com>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  1 sibling, 0 replies; 18+ messages in thread

From: Suraj Khamkar @ 2021-08-11 17:34 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers@lists.postgresql.org; michael@paquier.xyz

Thanks Dagfinn for the updated patches.

I do not get these errors, neither with the patch file I still have
> locally, or by saving the attachment from my original email.  Are you
> sure something in your download process hasn't converted it to Windows
> line endings (\r\n), or otherwise mangled the whitespace?


No. I have downloaded patch on linux and patch also doesn't have any
trailing
spaces, though it is throwing an error.

Here are few other comments,

   1. USER MAPPING does not have SESSION_USER as username in syntax
   (though it works) but your changes provide the same. Also, we have USER
    in
   list which is missing in current code changes.
   CREATE USER MAPPING [ IF NOT EXISTS ] FOR { user_name | USER |
   CURRENT_ROLE | CURRENT_USER | PUBLIC }
       SERVER server_name
       [ OPTIONS ( option 'value' [ , ... ] ) ]
   2. It might not be a scope of this ticket but as we are changing the
   query for ALTER GROUP,
   we should complete the role_specification after ALTER GROUP.
   postgres@17077=#ALTER GROUP
   pg_database_owner          pg_monitor
   pg_read_all_settings
   pg_read_server_files       pg_stat_scan_tables
    pg_write_server_files
   surajkhamkar.              pg_execute_server_program  pg_read_all_data

   pg_read_all_stats          pg_signal_backend          pg_write_all_data

   3. Missing schema_elements after CREATE SCHEMA AUTHORIZATION username to
   tab-complete .
   schema_elements might be CREATE, GRAND etc.


Thanks..

On Mon, Aug 9, 2021 at 11:30 PM Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
wrote:

> Hi Suraj,
>
> Suraj Khamkar <khamkarsuraj.b@gmail.com> writes:
>
> > Hello Dagfinn,
> >
> > I had a look at your patch and below are my review comments.
> > Please correct me if I am missing something.
> >
> >    1. For me the patch does not apply cleanly. I have been facing the
> error
> >    of trailing whitespaces.
>
> I do not get these errors, neither with the patch file I still have
> locally, or by saving the attachment from my original email.  Are you
> sure something in your download process hasn't converted it to Windows
> line endings (\r\n), or otherwise mangled the whitespace?
>
> >    2. We can remove space in before \ and below change
> >    +" UNION ALL SELECT 'PUBLIC'" \
> >
> >    Should be,
> >    +" UNION ALL SELECT 'PUBLIC' "\
>
> The patch doesn't add any lines that end with quote-space-backslash.
> As for the space before the quote, that is not necessary either, since the
> next line starts with a space after the quote.  Either way, the updated
> version of the patch doesn't add any new lines with continuation after a
> string constant, so the point is moot.
>
> >    3. role_specification has CURRENT_ROLE, CURRENT_USER and SESSION_USER.
> >    But current changes are missing CURRENT_ROLE.
>
> Ah, I was looking at the documentation for 13, but CURRENT_ROLE is only
> allowed in this context as of 14.  Fixed.
>
> >    4. I'm not sure about this but do we need to enable tab completion
> for IF
> >    NOT EXIST?
> >
> >    5. I think we are not handling IF NOT EXIST that's why it's not
> >    completing tab completion
> >    for AUTHORIZATION.
>
> As you note, psql currently doesn't currently tab-complete IF NOT EXISTS
> for any command, so that would be a subject for a separate patch.
>
> >    6. As we are here we can also enable missing tab completion for ALTER
> >    SCHEMA.
> >    After OWNER TO we should also get CURRENT_ROLE, CURRENT_USER and
> >    SESSION_USER.
>
> I did an audit of all the uses of Query_for_list_of_roles, and there
> turned out be several more that accept CURRENT_ROLE, CURRENT_USER and
> SESSION_USER that they weren't tab-completed for.  I also renamed the
> constant to Query_for_list_of_owner_roles, but I'm not 100% happy with
> that name either.
>
> >    7. Similarly, as we can drop multiple schemas' simultaneously, we
> should
> >    enable tab completion for
> >    comma with CASCADE and RESTRICT
> >    postgres@53724=#DROP SCHEMA sch
> >    CASCADE   RESTRICT
>
> The tab completion code for DROP is generic for all object types (see
> the words_after_create array and the create_or_drop_command_generator
> function), so that should be done genericallly, and is thus outside the
> scope for this patch.
>
> > Thanks.
>
> Thanks for the review.  Updated patch attached, with the CURRENT/SESSION
> ROLE/USER changes for other commands separated out.
>
> - ilmari
>
>

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2021-12-03 01:19  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2021-12-03 01:19 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Wed, Aug 11, 2021 at 10:16:15AM +0900, Michael Paquier wrote:
> +       else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION"))
> +               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
> +       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION"))
> +               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
> +       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
> +               COMPLETE_WITH("CREATE", "GRANT");
> +       else if (Matches("CREATE", "SCHEMA", MatchAny))
> +               COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
> Looks like you forgot the case "CREATE SCHEMA AUTHORIZATION MatchAny"
> that should be completed by GRANT and CREATE.

This patch has been waiting on author for more than a couple of weeks,
so I have marked it as returned with feedback in the CF app.  Please
feel free to resubmit if you are able to work more on that.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-04-14 16:04  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-04-14 16:04 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:

> On Wed, Aug 11, 2021 at 10:16:15AM +0900, Michael Paquier wrote:
>> +       else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION"))
>> +               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
>> +       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION"))
>> +               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
>> +       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
>> +               COMPLETE_WITH("CREATE", "GRANT");
>> +       else if (Matches("CREATE", "SCHEMA", MatchAny))
>> +               COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
>> Looks like you forgot the case "CREATE SCHEMA AUTHORIZATION MatchAny"
>> that should be completed by GRANT and CREATE.
>
> This patch has been waiting on author for more than a couple of weeks,
> so I have marked it as returned with feedback in the CF app.  Please
> feel free to resubmit if you are able to work more on that.

Looks like I completely dropped the ball on this one, sorry.  Here's a
rebased patch which uses the new COMPLETE_WITH_QUERY_PLUS functionality
added in commit 02b8048ba5dc36238f3e7c3c58c5946220298d71.


- ilmari

Attachments:

v2-0001-Add-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 2.7K) download From 4c4833dfdd2bb01cd35715223433f961e5ec004c Mon Sep 17 00:00:00 2001 From: tanghy <tanghy.fnst@fujitsu.com> Date: Mon, 9 Aug 2021 18:47:12 +0100 Subject: [PATCH v2] Add tab completion for CREATE SCHEMA - AUTHORIZATION both in addition to and after a schema name - list of owner roles after AUTHORIZATION - CREATE and GRANT after any otherwise-complete command --- src/bin/psql/tab-complete.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 5825b2a195..222dd617a2 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1031,9 +1031,13 @@ static const SchemaQuery Query_for_trigger_of_table = { " FROM pg_catalog.pg_roles "\ " WHERE rolname LIKE '%s'" +/* add these to Query_for_list_of_roles in OWNER contexts */ +#define Keywords_for_list_of_owner_roles \ +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" + /* add these to Query_for_list_of_roles in GRANT contexts */ #define Keywords_for_list_of_grant_roles \ -"PUBLIC", "CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" +Keywords_for_list_of_owner_roles, "PUBLIC" #define Query_for_all_table_constraints \ "SELECT conname "\ @@ -3154,6 +3158,20 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas, + "AUTHORIZATION"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION") || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, + Keywords_for_list_of_owner_roles); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) @@ -4263,9 +4281,7 @@ psql_completion(const char *text, int start, int end) /* OWNER TO - complete with available roles */ else if (TailMatches("OWNER", "TO")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, - "CURRENT_ROLE", - "CURRENT_USER", - "SESSION_USER"); + Keywords_for_list_of_owner_roles); /* ORDER BY */ else if (TailMatches("FROM", MatchAny, "ORDER")) -- 2.39.2
^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-04-15 02:06  Michael Paquier <michael@paquier.xyz>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2023-04-15 02:06 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Fri, Apr 14, 2023 at 05:04:35PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Looks like I completely dropped the ball on this one, sorry.

So did I ;)

> Here's a
> rebased patch which uses the new COMPLETE_WITH_QUERY_PLUS functionality
> added in commit 02b8048ba5dc36238f3e7c3c58c5946220298d71.

Thanks, I'll look at it.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-02 02:09  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2023-05-02 02:09 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Sat, Apr 15, 2023 at 11:06:25AM +0900, Michael Paquier wrote:
> Thanks, I'll look at it.

+   else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) ||
+            Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
+       COMPLETE_WITH("CREATE", "GRANT");
+   else if (Matches("CREATE", "SCHEMA", MatchAny))
+       COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");

I had this grammar under my eyes a few days ago for a different patch,
and there are much more objects types that can be appended to a CREATE
SCHEMA, like triggers, sequences, tables or views, so this is
incomplete, isn't it?
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-02 10:48  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-05-02 10:48 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:

> On Sat, Apr 15, 2023 at 11:06:25AM +0900, Michael Paquier wrote:
>> Thanks, I'll look at it.
>
> +   else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) ||
> +            Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
> +       COMPLETE_WITH("CREATE", "GRANT");
> +   else if (Matches("CREATE", "SCHEMA", MatchAny))
> +       COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
>
> I had this grammar under my eyes a few days ago for a different patch,
> and there are much more objects types that can be appended to a CREATE
> SCHEMA, like triggers, sequences, tables or views, so this is
> incomplete, isn't it?

This is for completing the word CREATE itself after CREATE SCHEMA
[[<name>] AUTHORIZATION] <name>.  The things that can come after that
are already handled generically earlier in the function:

/* CREATE */
    /* complete with something you can create */
    else if (TailMatches("CREATE"))
        matches = rl_completion_matches(text, create_command_generator);

create_command_generator uses the words_after_create array, which lists
all the things that can be created.

- ilmari





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-02 12:19  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-05-02 12:19 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:

> Michael Paquier <michael@paquier.xyz> writes:
>
>> On Sat, Apr 15, 2023 at 11:06:25AM +0900, Michael Paquier wrote:
>>> Thanks, I'll look at it.
>>
>> +   else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) ||
>> +            Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
>> +       COMPLETE_WITH("CREATE", "GRANT");
>> +   else if (Matches("CREATE", "SCHEMA", MatchAny))
>> +       COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
>>
>> I had this grammar under my eyes a few days ago for a different patch,
>> and there are much more objects types that can be appended to a CREATE
>> SCHEMA, like triggers, sequences, tables or views, so this is
>> incomplete, isn't it?
>
> This is for completing the word CREATE itself after CREATE SCHEMA
> [[<name>] AUTHORIZATION] <name>.  The things that can come after that
> are already handled generically earlier in the function:
>
> /* CREATE */
>     /* complete with something you can create */
>     else if (TailMatches("CREATE"))
>         matches = rl_completion_matches(text, create_command_generator);
>
> create_command_generator uses the words_after_create array, which lists
> all the things that can be created.

But, looking closer at the docs, only tables, views, indexes, sequences
and triggers can be created as part of a CREATE SCHEMA statement. Maybe
we should add a HeadMatches("CREATE", "SCHEMA") exception in the above?

- ilmari





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-06 02:58  Michael Paquier <michael@paquier.xyz>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2023-05-06 02:58 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Tue, May 02, 2023 at 01:19:49PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
>> This is for completing the word CREATE itself after CREATE SCHEMA
>> [[<name>] AUTHORIZATION] <name>.  The things that can come after that
>> are already handled generically earlier in the function:
>>
>> /* CREATE */
>>     /* complete with something you can create */
>>     else if (TailMatches("CREATE"))
>>         matches = rl_completion_matches(text, create_command_generator);
>>
>> create_command_generator uses the words_after_create array, which lists
>> all the things that can be created.

You are right.  I have completely forgotten that this code path would
append everything that supports CREATE for a CREATE SCHEMA command :)

> But, looking closer at the docs, only tables, views, indexes, sequences
> and triggers can be created as part of a CREATE SCHEMA statement. Maybe
> we should add a HeadMatches("CREATE", "SCHEMA") exception in the above?

Yes, it looks like we are going to need an exception and append only
the keywords that are supported, or we will end up recommending mostly
things that are not accepted by the parser.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-08 16:36  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-05-08 16:36 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:

> On Tue, May 02, 2023 at 01:19:49PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
>>> This is for completing the word CREATE itself after CREATE SCHEMA
>>> [[<name>] AUTHORIZATION] <name>.  The things that can come after that
>>> are already handled generically earlier in the function:
>>>
>>> /* CREATE */
>>>     /* complete with something you can create */
>>>     else if (TailMatches("CREATE"))
>>>         matches = rl_completion_matches(text, create_command_generator);
>>>
>>> create_command_generator uses the words_after_create array, which lists
>>> all the things that can be created.
>
> You are right.  I have completely forgotten that this code path would
> append everything that supports CREATE for a CREATE SCHEMA command :)
>
>> But, looking closer at the docs, only tables, views, indexes, sequences
>> and triggers can be created as part of a CREATE SCHEMA statement. Maybe
>> we should add a HeadMatches("CREATE", "SCHEMA") exception in the above?
>
> Yes, it looks like we are going to need an exception and append only
> the keywords that are supported, or we will end up recommending mostly
> things that are not accepted by the parser.

Here's an updated v3 patch with that.  While adding that, I noticed that
CREATE UNLOGGED only tab-completes TABLE and MATERIALIZED VIEW, not
SEQUENCE, so I added that (and removed MATERIALIZED VIEW when part of
CREATE SCHEMA).

- ilmari

Attachments:

v3-0001-Add-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 4.3K) download From 31856bf5397253da76cbce9ccb590855a44da30d Mon Sep 17 00:00:00 2001 From: tanghy <tanghy.fnst@fujitsu.com> Date: Mon, 9 Aug 2021 18:47:12 +0100 Subject: [PATCH v3] Add tab completion for CREATE SCHEMA - AUTHORIZATION both in addition to and after a schema name - list of owner roles after AUTHORIZATION - CREATE and GRANT after any otherwise-complete command - Only the allowed object types after CREATE Also adjust complation after CREATE UNLOGGED: - Add SEQUENCE - Don't suggest MATERIALIZED VIEW in CREATE TABLE --- src/bin/psql/tab-complete.c | 40 ++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index bd04244969..66b3f00c1b 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1031,9 +1031,13 @@ static const SchemaQuery Query_for_trigger_of_table = { " FROM pg_catalog.pg_roles "\ " WHERE rolname LIKE '%s'" +/* add these to Query_for_list_of_roles in OWNER contexts */ +#define Keywords_for_list_of_owner_roles \ +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" + /* add these to Query_for_list_of_roles in GRANT contexts */ #define Keywords_for_list_of_grant_roles \ -"PUBLIC", "CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" +Keywords_for_list_of_owner_roles, "PUBLIC" #define Query_for_all_table_constraints \ "SELECT conname "\ @@ -1785,7 +1789,13 @@ psql_completion(const char *text, int start, int end) /* CREATE */ /* complete with something you can create */ else if (TailMatches("CREATE")) - matches = rl_completion_matches(text, create_command_generator); + /* only some object types can be created as part of CREATE SCHEMA */ + if (HeadMatches("CREATE", "SCHEMA")) + COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER", + /* for INDEX and TABLE/SEQUENCE, respectively */ + "UNIQUE", "UNLOGGED"); + else + matches = rl_completion_matches(text, create_command_generator); /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -3154,6 +3164,20 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas, + "AUTHORIZATION"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION") || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, + Keywords_for_list_of_owner_roles); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) @@ -3185,9 +3209,13 @@ psql_completion(const char *text, int start, int end) /* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */ else if (TailMatches("CREATE", "TEMP|TEMPORARY")) COMPLETE_WITH("SEQUENCE", "TABLE", "VIEW"); - /* Complete "CREATE UNLOGGED" with TABLE or MATVIEW */ + /* Complete "CREATE UNLOGGED" with TABLE, SEQUENCE or MATVIEW */ else if (TailMatches("CREATE", "UNLOGGED")) - COMPLETE_WITH("TABLE", "MATERIALIZED VIEW"); + /* but not MATVIEW in CREATE SCHEMA */ + if (HeadMatches("CREATE", "SCHEMA")) + COMPLETE_WITH("TABLE", "SEQUENCE"); + else + COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW"); /* Complete PARTITION BY with RANGE ( or LIST ( or ... */ else if (TailMatches("PARTITION", "BY")) COMPLETE_WITH("RANGE (", "LIST (", "HASH ("); @@ -4263,9 +4291,7 @@ psql_completion(const char *text, int start, int end) /* OWNER TO - complete with available roles */ else if (TailMatches("OWNER", "TO")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, - "CURRENT_ROLE", - "CURRENT_USER", - "SESSION_USER"); + Keywords_for_list_of_owner_roles); /* ORDER BY */ else if (TailMatches("FROM", MatchAny, "ORDER")) -- 2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-05-09 03:26  Michael Paquier <michael@paquier.xyz>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2023-05-09 03:26 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Mon, May 08, 2023 at 05:36:27PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Here's an updated v3 patch with that.  While adding that, I noticed that
> CREATE UNLOGGED only tab-completes TABLE and MATERIALIZED VIEW, not
> SEQUENCE, so I added that (and removed MATERIALIZED VIEW when part of
> CREATE SCHEMA).

+       /* but not MATVIEW in CREATE SCHEMA */
+       if (HeadMatches("CREATE", "SCHEMA"))
+           COMPLETE_WITH("TABLE", "SEQUENCE");
+       else
+           COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW");

This may look strange at first glance, but the grammar is what it
is..  Perhaps matviews could be part of that at some point.  Or not. :)

+       /* only some object types can be created as part of CREATE SCHEMA */
+       if (HeadMatches("CREATE", "SCHEMA"))
+           COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER",
+                         /* for INDEX and TABLE/SEQUENCE, respectively */
+                         "UNIQUE", "UNLOGGED");

Not including TEMPORARY is OK here as the grammar does not allow a
directly to create a temporary schema.  The (many) code paths that
have TailMatches() to cope with CREATE SCHEMA would continue the
completion of added, but at least this approach avoids the
recommendation if possible.

That looks pretty much OK to me.  One tiny comment I have is that this
lacks brackets for the inner blocks, so I have added some in the v4
attached.
--
Michael

Attachments:

v4-0001-Add-tab-completion-for-CREATE-SCHEMA.patch (text/x-diff, 4.4K) download From d163ead2e81048af130ab4b72642b8e89d0c7e32 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@paquier.xyz> Date: Tue, 9 May 2023 12:25:03 +0900 Subject: [PATCH v4] Add tab completion for CREATE SCHEMA - AUTHORIZATION both in addition to and after a schema name - list of owner roles after AUTHORIZATION - CREATE and GRANT after any otherwise-complete command - Only the allowed object types after CREATE Also adjust complation after CREATE UNLOGGED: - Add SEQUENCE - Don't suggest MATERIALIZED VIEW in CREATE TABLE --- src/bin/psql/tab-complete.c | 45 ++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index bd04244969..9b0b7ddb33 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1031,9 +1031,13 @@ static const SchemaQuery Query_for_trigger_of_table = { " FROM pg_catalog.pg_roles "\ " WHERE rolname LIKE '%s'" +/* add these to Query_for_list_of_roles in OWNER contexts */ +#define Keywords_for_list_of_owner_roles \ +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" + /* add these to Query_for_list_of_roles in GRANT contexts */ #define Keywords_for_list_of_grant_roles \ -"PUBLIC", "CURRENT_ROLE", "CURRENT_USER", "SESSION_USER" +Keywords_for_list_of_owner_roles, "PUBLIC" #define Query_for_all_table_constraints \ "SELECT conname "\ @@ -1785,8 +1789,15 @@ psql_completion(const char *text, int start, int end) /* CREATE */ /* complete with something you can create */ else if (TailMatches("CREATE")) - matches = rl_completion_matches(text, create_command_generator); - + { + /* only some object types can be created as part of CREATE SCHEMA */ + if (HeadMatches("CREATE", "SCHEMA")) + COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER", + /* for INDEX and TABLE/SEQUENCE, respectively */ + "UNIQUE", "UNLOGGED"); + else + matches = rl_completion_matches(text, create_command_generator); + } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) COMPLETE_WITH("FUNCTION", "PROCEDURE", "LANGUAGE", "RULE", "VIEW", @@ -3154,6 +3165,20 @@ psql_completion(const char *text, int start, int end) else if (TailMatches("AS", "ON", "SELECT|UPDATE|INSERT|DELETE", "TO")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); +/* CREATE SCHEMA [ <name> ] [ AUTHORIZATION ] */ + else if (Matches("CREATE", "SCHEMA")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas, + "AUTHORIZATION"); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION") || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION")) + COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, + Keywords_for_list_of_owner_roles); + else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION", MatchAny) || + Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny)) + COMPLETE_WITH("CREATE", "GRANT"); + else if (Matches("CREATE", "SCHEMA", MatchAny)) + COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT"); + /* CREATE SEQUENCE --- is allowed inside CREATE SCHEMA, so use TailMatches */ else if (TailMatches("CREATE", "SEQUENCE", MatchAny) || TailMatches("CREATE", "TEMP|TEMPORARY", "SEQUENCE", MatchAny)) @@ -3185,9 +3210,15 @@ psql_completion(const char *text, int start, int end) /* Complete "CREATE TEMP/TEMPORARY" with the possible temp objects */ else if (TailMatches("CREATE", "TEMP|TEMPORARY")) COMPLETE_WITH("SEQUENCE", "TABLE", "VIEW"); - /* Complete "CREATE UNLOGGED" with TABLE or MATVIEW */ + /* Complete "CREATE UNLOGGED" with TABLE, SEQUENCE or MATVIEW */ else if (TailMatches("CREATE", "UNLOGGED")) - COMPLETE_WITH("TABLE", "MATERIALIZED VIEW"); + { + /* but not MATVIEW in CREATE SCHEMA */ + if (HeadMatches("CREATE", "SCHEMA")) + COMPLETE_WITH("TABLE", "SEQUENCE"); + else + COMPLETE_WITH("TABLE", "SEQUENCE", "MATERIALIZED VIEW"); + } /* Complete PARTITION BY with RANGE ( or LIST ( or ... */ else if (TailMatches("PARTITION", "BY")) COMPLETE_WITH("RANGE (", "LIST (", "HASH ("); @@ -4263,9 +4294,7 @@ psql_completion(const char *text, int start, int end) /* OWNER TO - complete with available roles */ else if (TailMatches("OWNER", "TO")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles, - "CURRENT_ROLE", - "CURRENT_USER", - "SESSION_USER"); + Keywords_for_list_of_owner_roles); /* ORDER BY */ else if (TailMatches("FROM", MatchAny, "ORDER")) -- 2.40.1
signature.asc (application/pgp-signature, 832B) download ^ permalink raw reply [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-06-30 01:36  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2023-06-30 01:36 UTC (permalink / raw)
  To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

On Tue, May 09, 2023 at 12:26:16PM +0900, Michael Paquier wrote:
> That looks pretty much OK to me.  One tiny comment I have is that this
> lacks brackets for the inner blocks, so I have added some in the v4
> attached.

The indentation was a bit wrong, so fixed it, and applied on HEAD.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Tab completion for CREATE SCHEMAAUTHORIZATION
@ 2023-06-30 09:33  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 18+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2023-06-30 09:33 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Suraj Khamkar <khamkarsuraj.b@gmail.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:

> On Tue, May 09, 2023 at 12:26:16PM +0900, Michael Paquier wrote:
>> That looks pretty much OK to me.  One tiny comment I have is that this
>> lacks brackets for the inner blocks, so I have added some in the v4
>> attached.
>
> The indentation was a bit wrong, so fixed it, and applied on HEAD.

Thanks!

- ilmari





^ permalink  raw  reply  [nested|flat] 18+ messages in thread


end of thread, other threads:[~2023-06-30 09:33 UTC | newest]

Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-09 16:20 Tab completion for CREATE SCHEMAAUTHORIZATION Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-07-15 11:13 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-07 21:09   ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-09 09:23     ` Suraj Khamkar <khamkarsuraj.b@gmail.com>
2021-08-09 18:00       ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-11 01:16         ` Michael Paquier <michael@paquier.xyz>
2021-12-03 01:19           ` Michael Paquier <michael@paquier.xyz>
2023-04-14 16:04             ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-04-15 02:06               ` Michael Paquier <michael@paquier.xyz>
2023-05-02 02:09                 ` Michael Paquier <michael@paquier.xyz>
2023-05-02 10:48                   ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-05-02 12:19                     ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-05-06 02:58                       ` Michael Paquier <michael@paquier.xyz>
2023-05-08 16:36                         ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-05-09 03:26                           ` Michael Paquier <michael@paquier.xyz>
2023-06-30 01:36                             ` Michael Paquier <michael@paquier.xyz>
2023-06-30 09:33                               ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-11 17:34         ` Suraj Khamkar <khamkarsuraj.b@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox