On Thu, 10 Jul 2025 13:23:47 +0900
Fujii Masao <masao.fu...@oss.nttdata.com> wrote:

> 
> 
> On 2025/07/10 10:30, Yugo Nagata wrote:
> > You're right. I must have overlooked something. I think I saw "TO" being
> > suggested after "FOREIGN SERVER" when no foreign servers were defined.
> > 
> > The attached patch still prevents "TO/FROM" from being suggested after
> > "FOREIGN SERVER" in such cases.

> 
> Based on your patch, I'm thinking of simplifying the code like this:
> 
> -       else if (Matches("GRANT", MatchAnyN, "ON", MatchAny, MatchAny))
> -               COMPLETE_WITH("TO");
> -       else if (Matches("REVOKE", MatchAnyN, "ON", MatchAny, MatchAny))
> -               COMPLETE_WITH("FROM");
> +       else if (Matches("GRANT|REVOKE", MatchAnyN, "ON", MatchAny, MatchAny) 
> &&
> +                        !TailMatches("FOREIGN", "SERVER") && 
> !TailMatches("LARGE", "OBJECT"))
> +       {
> +               if (Matches("GRANT", MatchAnyN, "ON", MatchAny, MatchAny))
> +                       COMPLETE_WITH("TO");
> +               else
> +                       COMPLETE_WITH("FROM");
> +       }

Thank you for your review!
 I agree with your suggestion and have updated the patch accordingly.

> 
> > But perhaps this corner case doesn't really
> > need to be handled?
> 
> Probably I failed to get your point here. Could you clarify what you meant?

I'm sorry for not explaining it clearly.

Currently, TO or FROM could be suggested immediately after FOREIGN SERVER, but
only when no foreign servers are defined. When foreign servers do exist,
their names are correctly suggested instead, as expected.

The patch fixed the behavior so that TO or FROM are not suggested after FOREIGN 
SERVER,
even when no foreign servers are defined. However, I've started to wonder if 
it's worth
fixing such a corner case. What do you think?

Regards,
Yugo Nagata

-- 
Yugo Nagata <nag...@sraoss.co.jp>
>From 3bf87cdc51c7576833b283b97d96927dfff6a7b0 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Tue, 8 Apr 2025 12:15:45 +0900
Subject: [PATCH v4] psql: Some improvement of tab completion for GRANT/REVOKE

This prevents suggesting TO or FROM immediately after LARGE OBJECT,
as a largeobject id is expected in that position. It also avoids suggesting
TO or FROM after FOREIGN SERVER when no foreign servers are defined.
When foreign servers do exist, their names are already suggested.
---
 src/bin/psql/tab-complete.in.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 5ba45a0bcb3..845a1894e36 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -4603,10 +4603,14 @@ match_previous_words(int pattern_id,
 	else if (Matches("ALTER", "DEFAULT", "PRIVILEGES", MatchAnyN, "TO", MatchAny))
 		COMPLETE_WITH("WITH GRANT OPTION");
 	/* Complete "GRANT/REVOKE ... ON * *" with TO/FROM */
-	else if (Matches("GRANT", MatchAnyN, "ON", MatchAny, MatchAny))
-		COMPLETE_WITH("TO");
-	else if (Matches("REVOKE", MatchAnyN, "ON", MatchAny, MatchAny))
-		COMPLETE_WITH("FROM");
+	else if (Matches("GRANT|REVOKE", MatchAnyN, "ON", MatchAny, MatchAny) &&
+			 !TailMatches("FOREIGN", "SERVER") && !TailMatches("LARGE", "OBJECT"))
+	{
+		if (Matches("GRANT", MatchAnyN, "ON", MatchAny, MatchAny))
+			COMPLETE_WITH("TO");
+		else
+			COMPLETE_WITH("FROM");
+	}
 
 	/* Complete "GRANT/REVOKE * ON ALL * IN SCHEMA *" with TO/FROM */
 	else if (TailMatches("GRANT|REVOKE", MatchAny, "ON", "ALL", MatchAny, "IN", "SCHEMA", MatchAny) ||
-- 
2.43.0

Reply via email to