Fix some newly modified tab-complete changes

Started by shiy.fnst@fujitsu.comover 3 years ago12 messages

over 3 years ago

1 attachment(s)

Hi hackers,

I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should
be "ALL TABLES IN SCHEMA" in the following case.

postgres=# grant all on
ALL FUNCTIONS IN SCHEMA DATABASE FUNCTION PARAMETER SCHEMA TABLESPACE
ALL PROCEDURES IN SCHEMA DOMAIN information_schema. PROCEDURE SEQUENCE tbl
ALL ROUTINES IN SCHEMA FOREIGN DATA WRAPPER LANGUAGE public. TABLE TYPE
ALL SEQUENCES IN SCHEMA FOREIGN SERVER LARGE OBJECT ROUTINE TABLES IN SCHEMA

I found that it is related to the recent commit 790bf615dd, and maybe it's
better to fix it. I also noticed that some comments should be modified according
to this new syntax. Attach a patch to fix them.

Regards,
Shi yu

Attachments:

0001-Fix-some-newly-modified-tab-complete-changes-and-com.patchapplication/octet-stream; name=0001-Fix-some-newly-modified-tab-complete-changes-and-com.patchDownload

From 342618ef1d334cc7f295ae64ae4d73515d65d546 Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Tue, 27 Sep 2022 18:21:56 +0800
Subject: [PATCH] Fix some newly modified tab-complete changes and comments.

---
 src/backend/replication/logical/tablesync.c |  4 ++--
 src/bin/psql/tab-complete.c                 | 12 ++++++------
 src/test/subscription/t/031_column_list.pl  |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 9e52fc401c..b4a7b4b7f6 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -978,8 +978,8 @@ fetch_remote_table_info(char *nspname, char *relname,
 	 *
 	 * 2) one of the subscribed publications has puballtables set to true
 	 *
-	 * 3) one of the subscribed publications is declared as ALL TABLES IN
-	 * SCHEMA that includes this relation
+	 * 3) one of the subscribed publications is declared as TABLES IN SCHEMA
+	 * that includes this relation
 	 */
 	if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
 	{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index dbe89d7eb2..8d297eb01f 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1848,7 +1848,7 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER PUBLICATION <name> SET */
 	else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET"))
 		COMPLETE_WITH("(", "TABLES IN SCHEMA", "TABLE");
-	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "ALL", "TABLES", "IN", "SCHEMA"))
+	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "TABLES", "IN", "SCHEMA"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
 								 " AND nspname NOT LIKE E'pg\\\\_%%'",
 								 "CURRENT_SCHEMA");
@@ -2994,8 +2994,8 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR"))
 		COMPLETE_WITH("TABLE", "ALL TABLES", "TABLES IN SCHEMA");
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL"))
-		COMPLETE_WITH("TABLES", "TABLES IN SCHEMA");
-	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES"))
+		COMPLETE_WITH("TABLES");
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES"))
 		COMPLETE_WITH("IN SCHEMA", "WITH (");
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLE", MatchAny) && !ends_with(prev_wd, ','))
 		COMPLETE_WITH("WHERE (", "WITH (");
@@ -3017,11 +3017,11 @@ psql_completion(const char *text, int start, int end)
 	/*
 	 * Complete "CREATE PUBLICATION <name> FOR TABLES IN SCHEMA <schema>, ..."
 	 */
-	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES", "IN", "SCHEMA"))
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", "IN", "SCHEMA"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
 								 " AND nspname NOT LIKE E'pg\\\\_%%'",
 								 "CURRENT_SCHEMA");
-	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES", "IN", "SCHEMA", MatchAny) && (!ends_with(prev_wd, ',')))
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", "IN", "SCHEMA", MatchAny) && (!ends_with(prev_wd, ',')))
 		COMPLETE_WITH("WITH (");
 	/* Complete "CREATE PUBLICATION <name> [...] WITH" */
 	else if (HeadMatches("CREATE", "PUBLICATION") && TailMatches("WITH", "("))
@@ -3835,7 +3835,7 @@ psql_completion(const char *text, int start, int end)
 											"ALL PROCEDURES IN SCHEMA",
 											"ALL ROUTINES IN SCHEMA",
 											"ALL SEQUENCES IN SCHEMA",
-											"TABLES IN SCHEMA",
+											"ALL TABLES IN SCHEMA",
 											"DATABASE",
 											"DOMAIN",
 											"FOREIGN DATA WRAPPER",
diff --git a/src/test/subscription/t/031_column_list.pl b/src/test/subscription/t/031_column_list.pl
index 3e4bfc2178..ae022faa78 100644
--- a/src/test/subscription/t/031_column_list.pl
+++ b/src/test/subscription/t/031_column_list.pl
@@ -899,8 +899,8 @@ is( $node_subscriber->safe_psql('postgres', "SELECT * FROM test_mix_2"),
 	'all columns should be replicated');
 
 
-# TEST: With a table included in the publication which is FOR ALL TABLES
-# IN SCHEMA, it means replicate all columns.
+# TEST: With a table included in the publication which is FOR TABLES IN
+# SCHEMA, it means replicate all columns.
 
 $node_subscriber->safe_psql(
 	'postgres', qq(
-- 
2.31.1

Peter Smith

smithpb2250@gmail.com

over 3 years ago

In reply to: shiy.fnst@fujitsu.com (#1)

Re: Fix some newly modified tab-complete changes

On Tue, Sep 27, 2022 at 8:28 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

Hi hackers,

I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should
be "ALL TABLES IN SCHEMA" in the following case.

postgres=# grant all on
ALL FUNCTIONS IN SCHEMA DATABASE FUNCTION PARAMETER SCHEMA TABLESPACE
ALL PROCEDURES IN SCHEMA DOMAIN information_schema. PROCEDURE SEQUENCE tbl
ALL ROUTINES IN SCHEMA FOREIGN DATA WRAPPER LANGUAGE public. TABLE TYPE
ALL SEQUENCES IN SCHEMA FOREIGN SERVER LARGE OBJECT ROUTINE TABLES IN SCHEMA

I found that it is related to the recent commit 790bf615dd, and maybe it's
better to fix it. I also noticed that some comments should be modified according
to this new syntax. Attach a patch to fix them.

Thanks for the patch! Below are my review comments.

The patch looks good to me but I did find some other tab-completion
anomalies. IIUC these are unrelated to your work, but since I found
them while testing your patch I am reporting them here.

Perhaps you want to fix them in the same patch, or just raise them
again separately?

======

1. tab complete for CREATE PUBLICATION

I don’t think this is any new bug, but I found that it is possible to do this...

test_pub=# create publication p for ALL TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

or, even this...

test_pub=# create publication p for XXX TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

======

2. tab complete for GRANT

test_pub=# grant <tab>
ALL EXECUTE
pg_execute_server_program pg_read_server_files postgres
TRIGGER
ALTER SYSTEM GRANT pg_monitor
pg_signal_backend REFERENCES
TRUNCATE
CONNECT INSERT pg_read_all_data
pg_stat_scan_tables SELECT UPDATE
CREATE pg_checkpoint
pg_read_all_settings pg_write_all_data SET
USAGE
DELETE pg_database_owner
pg_read_all_stats pg_write_server_files TEMPORARY

2a.
grant "GRANT" ??

2b.
grant "TEMPORARY" but not "TEMP" ??

------
Kind Regards,
Peter Smith.
Fujitsu Australia.

Kyotaro Horiguchi

horikyota.ntt@gmail.com

over 3 years ago

In reply to: Peter Smith (#2)

Re: Fix some newly modified tab-complete changes

At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith <smithpb2250@gmail.com> wrote in

On Tue, Sep 27, 2022 at 8:28 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

Hi hackers,

I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should
be "ALL TABLES IN SCHEMA" in the following case.

postgres=# grant all on
ALL FUNCTIONS IN SCHEMA DATABASE FUNCTION PARAMETER SCHEMA TABLESPACE
ALL PROCEDURES IN SCHEMA DOMAIN information_schema. PROCEDURE SEQUENCE tbl
ALL ROUTINES IN SCHEMA FOREIGN DATA WRAPPER LANGUAGE public. TABLE TYPE
ALL SEQUENCES IN SCHEMA FOREIGN SERVER LARGE OBJECT ROUTINE TABLES IN SCHEMA

I found that it is related to the recent commit 790bf615dd, and maybe it's
better to fix it. I also noticed that some comments should be modified according
to this new syntax. Attach a patch to fix them.

Thanks for the patch! Below are my review comments.

The patch looks good to me but I did find some other tab-completion
anomalies. IIUC these are unrelated to your work, but since I found
them while testing your patch I am reporting them here.

Looks fine to me, too.

Perhaps you want to fix them in the same patch, or just raise them
again separately?

======

1. tab complete for CREATE PUBLICATION

I don’t think this is any new bug, but I found that it is possible to do this...

test_pub=# create publication p for ALL TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

or, even this...

test_pub=# create publication p for XXX TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

Completion is responding to "IN SCHEMA" in these cases. However, I
don't reach this state only by completion becuase it doesn't suggest
"IN SCHEMA" after "TABLES" nor "ALL TABLES". I don't see a reason to
change that behavior unless that fix doesn't cause any additional
complexity.

======

2. tab complete for GRANT

test_pub=# grant <tab>
ALL EXECUTE
pg_execute_server_program pg_read_server_files postgres
TRIGGER
ALTER SYSTEM GRANT pg_monitor
pg_signal_backend REFERENCES
TRUNCATE
CONNECT INSERT pg_read_all_data
pg_stat_scan_tables SELECT UPDATE
CREATE pg_checkpoint
pg_read_all_settings pg_write_all_data SET
USAGE
DELETE pg_database_owner
pg_read_all_stats pg_write_server_files TEMPORARY

2a.
grant "GRANT" ??

Yeah, for the mement I thought that might a kind of admin option but
there's no such a privilege. REVOKE gets the same suggestion.

2b.
grant "TEMPORARY" but not "TEMP" ??

TEMP is an alternative spelling so that's fine.

I found the following suggestion.

CREATE PUBLICATION p FOR TABLES <tab> -> ["IN SCHEMA", "WITH ("]

I believe "WITH (" doesn't come there.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

shiy.fnst@fujitsu.com

over 3 years ago

In reply to: Kyotaro Horiguchi (#3)

2 attachment(s)

RE: Fix some newly modified tab-complete changes

On Wed, Sep 28, 2022 1:49 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:

At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith
<smithpb2250@gmail.com> wrote in

On Tue, Sep 27, 2022 at 8:28 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

Hi hackers,

I saw a problem when using tab-complete for "GRANT", "TABLES IN

SCHEMA" should

be "ALL TABLES IN SCHEMA" in the following case.

postgres=# grant all on
ALL FUNCTIONS IN SCHEMA DATABASE FUNCTION

PARAMETER SCHEMA TABLESPACE

ALL PROCEDURES IN SCHEMA DOMAIN information_schema.

PROCEDURE SEQUENCE tbl

ALL ROUTINES IN SCHEMA FOREIGN DATA WRAPPER LANGUAGE

public. TABLE TYPE

ALL SEQUENCES IN SCHEMA FOREIGN SERVER LARGE OBJECT

ROUTINE TABLES IN SCHEMA

I found that it is related to the recent commit 790bf615dd, and maybe it's
better to fix it. I also noticed that some comments should be modified

according

to this new syntax. Attach a patch to fix them.

Thanks for the patch! Below are my review comments.

The patch looks good to me but I did find some other tab-completion
anomalies. IIUC these are unrelated to your work, but since I found
them while testing your patch I am reporting them here.

Looks fine to me, too.

Thanks for reviewing it.

Perhaps you want to fix them in the same patch, or just raise them
again separately?

======

1. tab complete for CREATE PUBLICATION

I donʼt think this is any new bug, but I found that it is possible to do this...

test_pub=# create publication p for ALL TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

or, even this...

test_pub=# create publication p for XXX TABLES IN SCHEMA <tab>
information_schema pg_catalog pg_toast public

Completion is responding to "IN SCHEMA" in these cases. However, I
don't reach this state only by completion becuase it doesn't suggest
"IN SCHEMA" after "TABLES" nor "ALL TABLES". I don't see a reason to
change that behavior unless that fix doesn't cause any additional
complexity.

======

2. tab complete for GRANT

test_pub=# grant <tab>
ALL EXECUTE
pg_execute_server_program pg_read_server_files postgres
TRIGGER
ALTER SYSTEM GRANT pg_monitor
pg_signal_backend REFERENCES
TRUNCATE
CONNECT INSERT pg_read_all_data
pg_stat_scan_tables SELECT UPDATE
CREATE pg_checkpoint
pg_read_all_settings pg_write_all_data SET
USAGE
DELETE pg_database_owner
pg_read_all_stats pg_write_server_files TEMPORARY

2a.
grant "GRANT" ??

Yeah, for the mement I thought that might a kind of admin option but
there's no such a privilege. REVOKE gets the same suggestion.

Maybe that's for "REVOKE GRANT OPTION FOR". But it is used by both GRANT and
REVOKE. I think it's a separate problem, I have tried to fix it in the attached
0002 patch.

2b.
grant "TEMPORARY" but not "TEMP" ??

TEMP is an alternative spelling so that's fine.

Agreed.

I found the following suggestion.

CREATE PUBLICATION p FOR TABLES <tab> -> ["IN SCHEMA", "WITH ("]

I believe "WITH (" doesn't come there.

Fixed.

Attach the updated patch.

Regards,
Shi yu

Attachments:

v2-0001-Fix-some-newly-modified-tab-complete-changes-and-.patchapplication/octet-stream; name=v2-0001-Fix-some-newly-modified-tab-complete-changes-and-.patchDownload

From b791e95bcaf9f67451609aba99bfdeba4b0481e1 Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Tue, 27 Sep 2022 18:21:56 +0800
Subject: [PATCH v2 1/2] Fix some newly modified tab-complete changes and
 comments.

The recent commit 790bf615dd removed ALL keyword from ALL TABLES IN SCHEMA for
publication, it miss-modified tab completion for GRANT/REVOKE. Fix it in this
patch. It also fixed tab completion about the nmodified synatax, and remove ALL
keyword in some comments.
---
 src/backend/replication/logical/tablesync.c |  4 ++--
 src/bin/psql/tab-complete.c                 | 14 ++++++++------
 src/test/subscription/t/031_column_list.pl  |  4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 9e52fc401c..b4a7b4b7f6 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -978,8 +978,8 @@ fetch_remote_table_info(char *nspname, char *relname,
 	 *
 	 * 2) one of the subscribed publications has puballtables set to true
 	 *
-	 * 3) one of the subscribed publications is declared as ALL TABLES IN
-	 * SCHEMA that includes this relation
+	 * 3) one of the subscribed publications is declared as TABLES IN SCHEMA
+	 * that includes this relation
 	 */
 	if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
 	{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index dbe89d7eb2..71cfe8aec1 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1848,7 +1848,7 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER PUBLICATION <name> SET */
 	else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET"))
 		COMPLETE_WITH("(", "TABLES IN SCHEMA", "TABLE");
-	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "ALL", "TABLES", "IN", "SCHEMA"))
+	else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "TABLES", "IN", "SCHEMA"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
 								 " AND nspname NOT LIKE E'pg\\\\_%%'",
 								 "CURRENT_SCHEMA");
@@ -2994,9 +2994,11 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR"))
 		COMPLETE_WITH("TABLE", "ALL TABLES", "TABLES IN SCHEMA");
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL"))
-		COMPLETE_WITH("TABLES", "TABLES IN SCHEMA");
+		COMPLETE_WITH("TABLES");
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES"))
-		COMPLETE_WITH("IN SCHEMA", "WITH (");
+		COMPLETE_WITH("WITH (");
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES"))
+		COMPLETE_WITH("IN SCHEMA");
 	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLE", MatchAny) && !ends_with(prev_wd, ','))
 		COMPLETE_WITH("WHERE (", "WITH (");
 	/* Complete "CREATE PUBLICATION <name> FOR TABLE" with "<table>, ..." */
@@ -3017,11 +3019,11 @@ psql_completion(const char *text, int start, int end)
 	/*
 	 * Complete "CREATE PUBLICATION <name> FOR TABLES IN SCHEMA <schema>, ..."
 	 */
-	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES", "IN", "SCHEMA"))
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", "IN", "SCHEMA"))
 		COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
 								 " AND nspname NOT LIKE E'pg\\\\_%%'",
 								 "CURRENT_SCHEMA");
-	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES", "IN", "SCHEMA", MatchAny) && (!ends_with(prev_wd, ',')))
+	else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", "IN", "SCHEMA", MatchAny) && (!ends_with(prev_wd, ',')))
 		COMPLETE_WITH("WITH (");
 	/* Complete "CREATE PUBLICATION <name> [...] WITH" */
 	else if (HeadMatches("CREATE", "PUBLICATION") && TailMatches("WITH", "("))
@@ -3835,7 +3837,7 @@ psql_completion(const char *text, int start, int end)
 											"ALL PROCEDURES IN SCHEMA",
 											"ALL ROUTINES IN SCHEMA",
 											"ALL SEQUENCES IN SCHEMA",
-											"TABLES IN SCHEMA",
+											"ALL TABLES IN SCHEMA",
 											"DATABASE",
 											"DOMAIN",
 											"FOREIGN DATA WRAPPER",
diff --git a/src/test/subscription/t/031_column_list.pl b/src/test/subscription/t/031_column_list.pl
index 3e4bfc2178..ae022faa78 100644
--- a/src/test/subscription/t/031_column_list.pl
+++ b/src/test/subscription/t/031_column_list.pl
@@ -899,8 +899,8 @@ is( $node_subscriber->safe_psql('postgres', "SELECT * FROM test_mix_2"),
 	'all columns should be replicated');
 
 
-# TEST: With a table included in the publication which is FOR ALL TABLES
-# IN SCHEMA, it means replicate all columns.
+# TEST: With a table included in the publication which is FOR TABLES IN
+# SCHEMA, it means replicate all columns.
 
 $node_subscriber->safe_psql(
 	'postgres', qq(
-- 
2.31.1

v2-0002-Fix-tab-completion-for-GRANT-REVOKE.patchapplication/octet-stream; name=v2-0002-Fix-tab-completion-for-GRANT-REVOKE.patchDownload

From 7ff8f79d4295042e09efdc8a0c16ccd660968cf5 Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Wed, 28 Sep 2022 17:59:17 +0800
Subject: [PATCH v2 2/2] Fix tab completion for GRANT/REVOKE

The result of tab completion for GRANT contains GRANT, but there's no such a
privilege. Fix it in this patch.
---
 src/bin/psql/tab-complete.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 71cfe8aec1..dbd34e4987 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3749,9 +3749,29 @@ psql_completion(const char *text, int start, int end)
 			COMPLETE_WITH("SELECT", "INSERT", "UPDATE",
 						  "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
 						  "EXECUTE", "USAGE", "ALL");
-		else
+		else if (TailMatches("REVOKE", "GRANT"))
+			COMPLETE_WITH("OPTION FOR");
+		else if (TailMatches("GRANT") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
+			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 "SELECT",
+									 "INSERT",
+									 "UPDATE",
+									 "DELETE",
+									 "TRUNCATE",
+									 "REFERENCES",
+									 "TRIGGER",
+									 "CREATE",
+									 "CONNECT",
+									 "TEMPORARY",
+									 "EXECUTE",
+									 "USAGE",
+									 "SET",
+									 "ALTER SYSTEM",
+									 "ALL");
+		else if (TailMatches("REVOKE"))
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
-									 "GRANT",
+									 "GRANT OPTION FOR",
 									 "SELECT",
 									 "INSERT",
 									 "UPDATE",
@@ -3769,8 +3789,6 @@ psql_completion(const char *text, int start, int end)
 									 "ALL");
 	}
 
-	else if (TailMatches("REVOKE", "GRANT"))
-		COMPLETE_WITH("OPTION FOR");
 	else if (TailMatches("REVOKE", "GRANT", "OPTION"))
 		COMPLETE_WITH("FOR");
 
-- 
2.31.1

Alvaro Herrera

alvherre@alvh.no-ip.org

over 3 years ago

In reply to: shiy.fnst@fujitsu.com (#4)

Re: Fix some newly modified tab-complete changes

Thanks! I pushed 0001.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)

Peter Smith

smithpb2250@gmail.com

over 3 years ago

In reply to: shiy.fnst@fujitsu.com (#4)

Re: Fix some newly modified tab-complete changes

On Thu, Sep 29, 2022 at 12:50 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

On Wed, Sep 28, 2022 1:49 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:

At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith
<smithpb2250@gmail.com> wrote in

...

2. tab complete for GRANT

test_pub=# grant <tab>
ALL EXECUTE
pg_execute_server_program pg_read_server_files postgres
TRIGGER
ALTER SYSTEM GRANT pg_monitor
pg_signal_backend REFERENCES
TRUNCATE
CONNECT INSERT pg_read_all_data
pg_stat_scan_tables SELECT UPDATE
CREATE pg_checkpoint
pg_read_all_settings pg_write_all_data SET
USAGE
DELETE pg_database_owner
pg_read_all_stats pg_write_server_files TEMPORARY

2a.
grant "GRANT" ??

Yeah, for the mement I thought that might a kind of admin option but
there's no such a privilege. REVOKE gets the same suggestion.

Maybe that's for "REVOKE GRANT OPTION FOR". But it is used by both GRANT and
REVOKE. I think it's a separate problem, I have tried to fix it in the attached
0002 patch.

I checked your v2-0002 patch and AFAICT it does fix properly the
previously reported GRANT/REVOKE problem.

But, while testing I noticed another different quirk

It seems that neither the GRANT nor the REVOKE auto-complete
recognises the optional PRIVILEGE keyword

e.g. GRANT ALL <tab> --> ON (but not PRIVILEGE)
e.g. GRANT ALL PRIV<tab> --> ???

e.g. REVOKE ALL <tab> --> ON (but not PRIVILEGE)..
e.g. REVOKE ALL PRIV<tab> --> ???

------
Kind Regards,
Peter Smith.
Fujitsu Australia

shiy.fnst@fujitsu.com

over 3 years ago

In reply to: Peter Smith (#6)

1 attachment(s)

RE: Fix some newly modified tab-complete changes

On Tue, Oct 4, 2022 4:17 PM Peter Smith <smithpb2250@gmail.com> wrote:

On Thu, Sep 29, 2022 at 12:50 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

On Wed, Sep 28, 2022 1:49 PM Kyotaro Horiguchi

<horikyota.ntt@gmail.com> wrote:

At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith
<smithpb2250@gmail.com> wrote in

...

2. tab complete for GRANT

test_pub=# grant <tab>
ALL EXECUTE
pg_execute_server_program pg_read_server_files postgres
TRIGGER
ALTER SYSTEM GRANT pg_monitor
pg_signal_backend REFERENCES
TRUNCATE
CONNECT INSERT pg_read_all_data
pg_stat_scan_tables SELECT UPDATE
CREATE pg_checkpoint
pg_read_all_settings pg_write_all_data SET
USAGE
DELETE pg_database_owner
pg_read_all_stats pg_write_server_files TEMPORARY

2a.
grant "GRANT" ??

Yeah, for the mement I thought that might a kind of admin option but
there's no such a privilege. REVOKE gets the same suggestion.

Maybe that's for "REVOKE GRANT OPTION FOR". But it is used by both

GRANT and

REVOKE. I think it's a separate problem, I have tried to fix it in the attached
0002 patch.

I checked your v2-0002 patch and AFAICT it does fix properly the
previously reported GRANT/REVOKE problem.

Thanks for reviewing and testing it.

~

But, while testing I noticed another different quirk

It seems that neither the GRANT nor the REVOKE auto-complete
recognises the optional PRIVILEGE keyword

e.g. GRANT ALL <tab> --> ON (but not PRIVILEGE)
e.g. GRANT ALL PRIV<tab> --> ???

e.g. REVOKE ALL <tab> --> ON (but not PRIVILEGE)..
e.g. REVOKE ALL PRIV<tab> --> ???

I tried to add tab-completion for it. Pleases see attached updated patch.

Regards,
Shi yu

Attachments:

v3-0001-Fix-tab-completion-for-GRANT-REVOKE.patchapplication/octet-stream; name=v3-0001-Fix-tab-completion-for-GRANT-REVOKE.patchDownload

From d435b5737f2db185072aa8161ec6dbd3b6e32b3e Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Wed, 28 Sep 2022 17:59:17 +0800
Subject: [PATCH v3] Fix tab completion for GRANT/REVOKE

The result of tab completion for GRANT contains GRANT, but there's no such a
privilege. Fix it in this patch. Also add tab completion for
GRANT/REVOKE ALL PRIVILEGES.
---
 src/bin/psql/tab-complete.c | 70 +++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 15 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 584d9d5ae6..fe01ca6019 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3749,9 +3749,29 @@ psql_completion(const char *text, int start, int end)
 			COMPLETE_WITH("SELECT", "INSERT", "UPDATE",
 						  "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
 						  "EXECUTE", "USAGE", "ALL");
-		else
+		else if (TailMatches("REVOKE", "GRANT"))
+			COMPLETE_WITH("OPTION FOR");
+		else if (TailMatches("GRANT") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
-									 "GRANT",
+									 "SELECT",
+									 "INSERT",
+									 "UPDATE",
+									 "DELETE",
+									 "TRUNCATE",
+									 "REFERENCES",
+									 "TRIGGER",
+									 "CREATE",
+									 "CONNECT",
+									 "TEMPORARY",
+									 "EXECUTE",
+									 "USAGE",
+									 "SET",
+									 "ALTER SYSTEM",
+									 "ALL");
+		else if (TailMatches("REVOKE"))
+			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 "GRANT OPTION FOR",
 									 "SELECT",
 									 "INSERT",
 									 "UPDATE",
@@ -3769,8 +3789,6 @@ psql_completion(const char *text, int start, int end)
 									 "ALL");
 	}
 
-	else if (TailMatches("REVOKE", "GRANT"))
-		COMPLETE_WITH("OPTION FOR");
 	else if (TailMatches("REVOKE", "GRANT", "OPTION"))
 		COMPLETE_WITH("FOR");
 
@@ -3807,13 +3825,18 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches("GRANT|REVOKE", MatchAny) ||
 			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny))
 	{
-		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
+		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE"))
 			COMPLETE_WITH("ON");
+		else if (TailMatches("ALL"))
+			COMPLETE_WITH("ON", "PRIVILEGES");
 		else if (TailMatches("GRANT", MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
 	}
+	else if (TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES"))
+		COMPLETE_WITH("ON");
 
 	/*
 	 * Complete GRANT/REVOKE <sth> ON with a list of appropriate relations.
@@ -3823,7 +3846,9 @@ psql_completion(const char *text, int start, int end)
 	 * privilege.
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON") )
 	{
 		/*
 		 * With ALTER DEFAULT PRIVILEGES, restrict completion to the kinds of
@@ -3855,14 +3880,18 @@ psql_completion(const char *text, int start, int end)
 											"TYPE");
 	}
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "ALL"))
 		COMPLETE_WITH("FUNCTIONS IN SCHEMA",
 					  "PROCEDURES IN SCHEMA",
 					  "ROUTINES IN SCHEMA",
 					  "SEQUENCES IN SCHEMA",
 					  "TABLES IN SCHEMA");
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN"))
 		COMPLETE_WITH("DATA WRAPPER", "SERVER");
 
 	/*
@@ -3872,7 +3901,9 @@ psql_completion(const char *text, int start, int end)
 	 * Complete "GRANT/REVOKE * ON *" with "TO/FROM".
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", MatchAny))
 	{
 		if (TailMatches("DATABASE"))
 			COMPLETE_WITH_QUERY(Query_for_list_of_databases);
@@ -3938,9 +3969,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", "ALL", "PRIVILEGES", "IN", "SCHEMA", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3948,9 +3982,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN DATA WRAPPER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny)
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3958,9 +3995,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN SERVER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
-- 
2.24.0.windows.2

shiy.fnst@fujitsu.com

over 3 years ago

In reply to: shiy.fnst@fujitsu.com (#7)

1 attachment(s)

RE: Fix some newly modified tab-complete changes

On Mon, Oct 10, 2022 2:12 PM shiy.fnst@fujitsu.com <shiy.fnst@fujitsu.com> wrote:

On Tue, Oct 4, 2022 4:17 PM Peter Smith <smithpb2250@gmail.com> wrote:

But, while testing I noticed another different quirk

It seems that neither the GRANT nor the REVOKE auto-complete
recognises the optional PRIVILEGE keyword

e.g. GRANT ALL <tab> --> ON (but not PRIVILEGE)
e.g. GRANT ALL PRIV<tab> --> ???

e.g. REVOKE ALL <tab> --> ON (but not PRIVILEGE)..
e.g. REVOKE ALL PRIV<tab> --> ???

I tried to add tab-completion for it. Pleases see attached updated patch.

Sorry for attaching a wrong patch. Here is the right one.

Regards,
Shi yu

Attachments:

v4-0001-Fix-tab-completion-for-GRANT-REVOKE.patchapplication/octet-stream; name=v4-0001-Fix-tab-completion-for-GRANT-REVOKE.patchDownload

From 4528cfd29bd7675d3c0ec426aac55857d7678085 Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Wed, 28 Sep 2022 17:59:17 +0800
Subject: [PATCH v4] Fix tab completion for GRANT/REVOKE

The result of tab completion for GRANT contains GRANT, but there's no such a
privilege. Fix it in this patch. Also add tab completion for
GRANT/REVOKE ALL PRIVILEGES.
---
 src/bin/psql/tab-complete.c | 70 +++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 15 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 584d9d5ae6..ac5ee56182 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3749,9 +3749,29 @@ psql_completion(const char *text, int start, int end)
 			COMPLETE_WITH("SELECT", "INSERT", "UPDATE",
 						  "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
 						  "EXECUTE", "USAGE", "ALL");
-		else
+		else if (TailMatches("REVOKE", "GRANT"))
+			COMPLETE_WITH("OPTION FOR");
+		else if (TailMatches("GRANT") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
-									 "GRANT",
+									 "SELECT",
+									 "INSERT",
+									 "UPDATE",
+									 "DELETE",
+									 "TRUNCATE",
+									 "REFERENCES",
+									 "TRIGGER",
+									 "CREATE",
+									 "CONNECT",
+									 "TEMPORARY",
+									 "EXECUTE",
+									 "USAGE",
+									 "SET",
+									 "ALTER SYSTEM",
+									 "ALL");
+		else if (TailMatches("REVOKE"))
+			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 "GRANT OPTION FOR",
 									 "SELECT",
 									 "INSERT",
 									 "UPDATE",
@@ -3769,8 +3789,6 @@ psql_completion(const char *text, int start, int end)
 									 "ALL");
 	}
 
-	else if (TailMatches("REVOKE", "GRANT"))
-		COMPLETE_WITH("OPTION FOR");
 	else if (TailMatches("REVOKE", "GRANT", "OPTION"))
 		COMPLETE_WITH("FOR");
 
@@ -3807,13 +3825,18 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches("GRANT|REVOKE", MatchAny) ||
 			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny))
 	{
-		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
+		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE"))
 			COMPLETE_WITH("ON");
+		else if (TailMatches("ALL"))
+			COMPLETE_WITH("ON", "PRIVILEGES");
 		else if (TailMatches("GRANT", MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
 	}
+	else if (TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES"))
+		COMPLETE_WITH("ON");
 
 	/*
 	 * Complete GRANT/REVOKE <sth> ON with a list of appropriate relations.
@@ -3823,7 +3846,9 @@ psql_completion(const char *text, int start, int end)
 	 * privilege.
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON") )
 	{
 		/*
 		 * With ALTER DEFAULT PRIVILEGES, restrict completion to the kinds of
@@ -3855,14 +3880,18 @@ psql_completion(const char *text, int start, int end)
 											"TYPE");
 	}
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "ALL"))
 		COMPLETE_WITH("FUNCTIONS IN SCHEMA",
 					  "PROCEDURES IN SCHEMA",
 					  "ROUTINES IN SCHEMA",
 					  "SEQUENCES IN SCHEMA",
 					  "TABLES IN SCHEMA");
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN"))
 		COMPLETE_WITH("DATA WRAPPER", "SERVER");
 
 	/*
@@ -3872,7 +3901,9 @@ psql_completion(const char *text, int start, int end)
 	 * Complete "GRANT/REVOKE * ON *" with "TO/FROM".
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", MatchAny))
 	{
 		if (TailMatches("DATABASE"))
 			COMPLETE_WITH_QUERY(Query_for_list_of_databases);
@@ -3938,9 +3969,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", "ALL", "PRIVILEGES", "IN", "SCHEMA", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3948,9 +3982,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN DATA WRAPPER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3958,9 +3995,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN SERVER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
-- 
2.31.1

Peter Smith

smithpb2250@gmail.com

about 3 years ago

In reply to: shiy.fnst@fujitsu.com (#8)

Re: Fix some newly modified tab-complete changes

On Tue, Oct 11, 2022 at 1:28 AM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:

On Mon, Oct 10, 2022 2:12 PM shiy.fnst@fujitsu.com <shiy.fnst@fujitsu.com> wrote:

On Tue, Oct 4, 2022 4:17 PM Peter Smith <smithpb2250@gmail.com> wrote:

But, while testing I noticed another different quirk

It seems that neither the GRANT nor the REVOKE auto-complete
recognises the optional PRIVILEGE keyword

e.g. GRANT ALL <tab> --> ON (but not PRIVILEGE)
e.g. GRANT ALL PRIV<tab> --> ???

e.g. REVOKE ALL <tab> --> ON (but not PRIVILEGE)..
e.g. REVOKE ALL PRIV<tab> --> ???

I tried to add tab-completion for it. Pleases see attached updated patch.

Hi Shi-san,

I re-tested and confirm that the patch does indeed fix the quirk I'd
previously reported.

But, looking at the patch code, I don't know if it is the best way to
fix the problem or not. Someone with more experience of the
tab-complete module can judge that.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

#10

Michael Paquier

michael@paquier.xyz

about 3 years ago

In reply to: Peter Smith (#9)

Re: Fix some newly modified tab-complete changes

On Tue, Oct 18, 2022 at 05:17:32PM +1100, Peter Smith wrote:

I re-tested and confirm that the patch does indeed fix the quirk I'd
previously reported.

But, looking at the patch code, I don't know if it is the best way to
fix the problem or not. Someone with more experience of the
tab-complete module can judge that.

It seems to me that the patch as proposed has more problems than
that. I have spotted a few issues at quick glance, there may be
more.

For example, take this one:
+       else if (TailMatches("GRANT") ||
+            TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
            COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,

"REVOKE GRANT OPTION FOR" completes with a list of role names, which
is incorrect.

FWIW, I am not much a fan of the approach taken by the patch to
duplicate the full list of keywords to append after REVOKE or GRANT,
at the only difference of "GRANT OPTION FOR". This may be readable if
unified with a single list, with extra items appended for GRANT and
REVOKE?

Note that REVOKE has a "ADMIN OPTION FOR" clause, which is not
completed to.
--
Michael

#11

shiy.fnst@fujitsu.com

about 3 years ago

In reply to: Michael Paquier (#10)

2 attachment(s)

RE: Fix some newly modified tab-complete changes

On Thu, Nov 10, 2022 12:54 PM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Oct 18, 2022 at 05:17:32PM +1100, Peter Smith wrote:

I re-tested and confirm that the patch does indeed fix the quirk I'd
previously reported.

But, looking at the patch code, I don't know if it is the best way to
fix the problem or not. Someone with more experience of the
tab-complete module can judge that.

It seems to me that the patch as proposed has more problems than
that. I have spotted a few issues at quick glance, there may be
more.
For example, take this one:
+       else if (TailMatches("GRANT") ||
+            TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
"REVOKE GRANT OPTION FOR" completes with a list of role names, which
is incorrect.

FWIW, I am not much a fan of the approach taken by the patch to
duplicate the full list of keywords to append after REVOKE or GRANT,
at the only difference of "GRANT OPTION FOR". This may be readable if
unified with a single list, with extra items appended for GRANT and
REVOKE?

Note that REVOKE has a "ADMIN OPTION FOR" clause, which is not
completed to.

Thanks a lot for looking into this patch.

I have fixed the problems you saw, and improved the patch as you suggested.

Besides, I noticed that the tab completion for "ALTER DEFAULT PRIVILEGES ...
GRANT/REVOKE ..." missed "CREATE". Fix it in 0001 patch.

And commit e3ce2de09 supported GRANT ... WITH INHERIT ..., but there's no tab
completion for it. Add this in 0002 patch.

Please see the attached patches.

Regards,
Shi yu

Attachments:

v5-0002-Add-tab-completion-for-GRANT-WITH-INHERIT.patchapplication/octet-stream; name=v5-0002-Add-tab-completion-for-GRANT-WITH-INHERIT.patchDownload

From 0d11e7cf810f281e42dc7bcfb439c9e9d6716e37 Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Wed, 16 Nov 2022 13:32:07 +0800
Subject: [PATCH v5 2/2] =?UTF-8?q?Add=20tab=20completion=20for=20GRANT=20?=
 =?UTF-8?q?=E2=80=A6=20WITH=20INHERIT=20=E2=80=A6?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/bin/psql/tab-complete.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 1f62ad401b..ff954e5c02 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3791,7 +3791,8 @@ psql_completion(const char *text, int start, int end)
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
 									 Privileges_options_of_grant_and_revoke,
 									 "GRANT OPTION FOR",
-									 "ADMIN OPTION FOR");
+									 "ADMIN OPTION FOR",
+									 "INHERIT OPTION FOR");
 		else if (TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
 			COMPLETE_WITH(Privileges_options_of_grant_and_revoke);
 	}
@@ -3953,12 +3954,17 @@ psql_completion(const char *text, int start, int end)
 	 * Offer grant options after that.
 	 */
 	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny))
-		COMPLETE_WITH("WITH ADMIN OPTION",
+		COMPLETE_WITH("WITH ADMIN",
+					  "WITH INHERIT",
 					  "WITH GRANT OPTION",
 					  "GRANTED BY");
 	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH"))
-		COMPLETE_WITH("ADMIN OPTION",
+		COMPLETE_WITH("ADMIN",
+					  "INHERIT",
 					  "GRANT OPTION");
+	else if (HeadMatches("GRANT") &&
+			 (TailMatches("TO", MatchAny, "WITH", "ADMIN|INHERIT")))
+		COMPLETE_WITH("OPTION", "TRUE", "FALSE");
 	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", MatchAny, "OPTION"))
 		COMPLETE_WITH("GRANTED BY");
 	else if (HeadMatches("GRANT") && TailMatches("TO", MatchAny, "WITH", MatchAny, "OPTION", "GRANTED", "BY"))
-- 
2.31.1

v5-0001-Fix-tab-completion-for-GRANT-REVOKE.patchapplication/octet-stream; name=v5-0001-Fix-tab-completion-for-GRANT-REVOKE.patchDownload

From c77d9fd2dbe3148b4041e6d60374253a8793a67d Mon Sep 17 00:00:00 2001
From: Shi Yu <shiy.fnst@fujitsu.com>
Date: Wed, 28 Sep 2022 17:59:17 +0800
Subject: [PATCH v5 1/2] Fix tab completion for GRANT/REVOKE

The result of tab completion for GRANT contains GRANT, but there's no such a
privilege. This is because the tab completions for GRANT and REVOKE are the
same. Fix it in this patch. Also add tab completion for GRANT/REVOKE ALL
PRIVILEGES.
---
 src/bin/psql/tab-complete.c | 82 +++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 31 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7b73886ce1..1f62ad401b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1143,6 +1143,12 @@ static const SchemaQuery Query_for_trigger_of_table = {
 "  FROM pg_catalog.pg_timezone_names() "\
 " WHERE pg_catalog.quote_literal(pg_catalog.lower(name)) LIKE pg_catalog.lower('%s')"
 
+/* Privileges options of GRANT and REVOKE*/
+#define Privileges_options_of_grant_and_revoke \
+"SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", \
+"CREATE", "CONNECT", "TEMPORARY", "EXECUTE", "USAGE", "SET", "ALTER SYSTEM", \
+"ALL"
+
 /*
  * These object types were introduced later than our support cutoff of
  * server version 9.2.  We use the VersionedQuery infrastructure so that
@@ -3775,29 +3781,21 @@ psql_completion(const char *text, int start, int end)
 		if (HeadMatches("ALTER", "DEFAULT", "PRIVILEGES"))
 			COMPLETE_WITH("SELECT", "INSERT", "UPDATE",
 						  "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
-						  "EXECUTE", "USAGE", "ALL");
-		else
+						  "CREATE", "EXECUTE", "USAGE", "ALL");
+		else if (TailMatches("REVOKE", "GRANT"))
+			COMPLETE_WITH("OPTION FOR");
+		else if (TailMatches("GRANT"))
 			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
-									 "GRANT",
-									 "SELECT",
-									 "INSERT",
-									 "UPDATE",
-									 "DELETE",
-									 "TRUNCATE",
-									 "REFERENCES",
-									 "TRIGGER",
-									 "CREATE",
-									 "CONNECT",
-									 "TEMPORARY",
-									 "EXECUTE",
-									 "USAGE",
-									 "SET",
-									 "ALTER SYSTEM",
-									 "ALL");
+									 Privileges_options_of_grant_and_revoke);
+		else if (TailMatches("REVOKE"))
+			COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,
+									 Privileges_options_of_grant_and_revoke,
+									 "GRANT OPTION FOR",
+									 "ADMIN OPTION FOR");
+		else if (TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
+			COMPLETE_WITH(Privileges_options_of_grant_and_revoke);
 	}
 
-	else if (TailMatches("REVOKE", "GRANT"))
-		COMPLETE_WITH("OPTION FOR");
 	else if (TailMatches("REVOKE", "GRANT", "OPTION"))
 		COMPLETE_WITH("FOR");
 
@@ -3834,13 +3832,18 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches("GRANT|REVOKE", MatchAny) ||
 			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny))
 	{
-		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
+		if (TailMatches("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE"))
 			COMPLETE_WITH("ON");
+		else if (TailMatches("ALL"))
+			COMPLETE_WITH("ON", "PRIVILEGES");
 		else if (TailMatches("GRANT", MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
 	}
+	else if (TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES"))
+		COMPLETE_WITH("ON");
 
 	/*
 	 * Complete GRANT/REVOKE <sth> ON with a list of appropriate relations.
@@ -3850,7 +3853,9 @@ psql_completion(const char *text, int start, int end)
 	 * privilege.
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON") )
 	{
 		/*
 		 * With ALTER DEFAULT PRIVILEGES, restrict completion to the kinds of
@@ -3882,14 +3887,18 @@ psql_completion(const char *text, int start, int end)
 											"TYPE");
 	}
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "ALL"))
 		COMPLETE_WITH("FUNCTIONS IN SCHEMA",
 					  "PROCEDURES IN SCHEMA",
 					  "ROUTINES IN SCHEMA",
 					  "SEQUENCES IN SCHEMA",
 					  "TABLES IN SCHEMA");
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN") ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN"))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN") ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN"))
 		COMPLETE_WITH("DATA WRAPPER", "SERVER");
 
 	/*
@@ -3899,7 +3908,9 @@ psql_completion(const char *text, int start, int end)
 	 * Complete "GRANT/REVOKE * ON *" with "TO/FROM".
 	 */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", MatchAny))
 	{
 		if (TailMatches("DATABASE"))
 			COMPLETE_WITH_QUERY(Query_for_list_of_databases);
@@ -3965,9 +3976,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "ALL", "ALL", "PRIVILEGES", "IN", "SCHEMA", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3975,9 +3989,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN DATA WRAPPER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "DATA", "WRAPPER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
@@ -3985,9 +4002,12 @@ psql_completion(const char *text, int start, int end)
 
 	/* Complete "GRANT/REVOKE * ON FOREIGN SERVER *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
-			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny))
+			 TailMatches("GRANT|REVOKE", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", MatchAny, "ON", "FOREIGN", "SERVER", MatchAny) ||
+			 TailMatches("REVOKE", "GRANT", "OPTION", "FOR", "ALL", "PRIVILEGES", "ON", "FOREIGN", "SERVER", MatchAny))
 	{
-		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
+		if (TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny) ||
+			TailMatches("GRANT", MatchAny, MatchAny, MatchAny, MatchAny, MatchAny, MatchAny))
 			COMPLETE_WITH("TO");
 		else
 			COMPLETE_WITH("FROM");
-- 
2.31.1

#12

Michael Paquier

michael@paquier.xyz

about 3 years ago

In reply to: shiy.fnst@fujitsu.com (#11)

Re: Fix some newly modified tab-complete changes

On Wed, Nov 16, 2022 at 08:29:24AM +0000, shiy.fnst@fujitsu.com wrote:

I have fixed the problems you saw, and improved the patch as you suggested.

Besides, I noticed that the tab completion for "ALTER DEFAULT PRIVILEGES ...
GRANT/REVOKE ..." missed "CREATE". Fix it in 0001 patch.

And commit e3ce2de09 supported GRANT ... WITH INHERIT ..., but there's no tab
completion for it. Add this in 0002 patch.

Thanks, I have been looking at the patch, and pondered about all the
bloat added by the handling of PRIVILEGES, to note at the end that ALL
PRIVILEGES is parsed the same way as ALL. So we don't actually need
any of the complications related to it and the result would be the
same.

I have merged 0001 and 0002 together, and applied the rest, which
looked rather fine. I have simplified as well a bit the parts where
"REVOKE GRANT" are specified in a row, to avoid fancy results in some
branches when we apply Privilege_options_of_grant_and_revoke.
--
Michael