On Wed, 9 Jul 2025 20:42:42 +0900
Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
>
>
> On 2025/06/11 13:57, Yugo Nagata wrote:
> > On Wed, 11 Jun 2025 13:33:07 +0900
> > Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
> >
> >>
> >>
> >> On 2025/06/11 11:49, Yugo Nagata wrote:
> >>> While looking at the thread [1], I've remembered this thread.
> >>> The patches in this thread are partially v18-related, but include
> >>> enhancement or fixes for existing feature, so should they be postponed
> >>> to v19, or should be separated properly to v18 part and other?
> >>>
> >>> [1]
> >>> https://www.postgresql.org/message-id/70372bdd-4399-4d5b-ab4f-6d4487a4911a%40oss.nttdata.com
> >>
> >> I see these patches more as enhancements to psql tab-completion,
> >> rather than fixes for clear oversights in the original commit.
> >>
> >> For example, if tab-completion for ALTER DEFAULT PRIVILEGES had
> >> completely missed LARGE OBJECTS, that would be an obvious oversight.
> >> But these patches go beyond that kind of issue.
> >>
> >> That said, if others think it's appropriate to include them in v18
> >> for consistency or completeness, I'm fine with that.
> >>
> >> Regarding the 0002 patch:
> >>
> >> - 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))
> >> + {
> >> + if (TailMatches("FOREIGN", "SERVER"))
> >> + COMPLETE_WITH_QUERY(Query_for_list_of_servers);
> >> + else if (!TailMatches("LARGE", "OBJECT"))
> >> + {
> >> + if (Matches("GRANT", MatchAnyN, "ON", MatchAny,
> >> MatchAny))
> >> + COMPLETE_WITH("TO");
> >> + else
> >> + COMPLETE_WITH("FROM");
> >> + }
> >> + }
> >>
> >> Wouldn't this change break the case where "GRANT ... ON TABLE ... <TAB>"
> >> is supposed to complete with "TO"?
> >
> > Sorry, I made a stupid mistake.
> >
> >> + else if (Matches("GRANT/REVOKE", MatchAnyN, "ON", MatchAny, MatchAny))
> >
> > This should be "GRANT|REVOKE".
> >
> > I've attached update patches. (There is no change on 0001.)
>
> Thanks for updating the patch! At first I've pushed the 0001 patch.
>
> As for the 0002 patch:
>
> + if (TailMatches("FOREIGN", "SERVER"))
> + COMPLETE_WITH_QUERY(Query_for_list_of_servers);
>
> This part seems not needed, since we already have the following
> tab-completion code:
>
> /* FOREIGN SERVER */
> else if (TailMatches("FOREIGN", "SERVER"))
> COMPLETE_WITH_QUERY(Query_for_list_of_servers);
>
> Thought?
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. But perhaps this corner case doesn't really
need to be handled?
Regards,
Yugo Nagata
--
Yugo Nagata <nag...@sraoss.co.jp>
>From 4ba9c4155ace519e016f26159e9587e6da488b86 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 v3] 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 | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 5ba45a0bcb3..ec5f50c64cb 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -4603,10 +4603,16 @@ 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))
+ {
+ if (!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
