Re: Miscellaneous tab completion issue fixes
On Wed, 5 Oct 2022 at 08:17, Michael Paquier wrote: > > On Tue, Oct 04, 2022 at 10:35:25AM +0100, Dagfinn Ilmari Mannsåker wrote: > > LGTM, +1 to commit. > > Fine by me, so done. Thanks for pushing this. Regards, Vignesh
Re: Miscellaneous tab completion issue fixes
On Tue, Oct 04, 2022 at 10:35:25AM +0100, Dagfinn Ilmari Mannsåker wrote: > LGTM, +1 to commit. Fine by me, so done. -- Michael signature.asc Description: PGP signature
Re: Miscellaneous tab completion issue fixes
vignesh C writes:
> On Tue, 4 Oct 2022 at 09:13, Michael Paquier wrote:
>>
>> On Mon, Oct 03, 2022 at 06:29:32PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> > vignesh C writes:
>> >> +else if (TailMatchesCS("\\dRp*"))
>> >> +COMPLETE_WITH_QUERY(Query_for_list_of_publications[0].query);
>> >> +else if (TailMatchesCS("\\dRs*"))
>> >> +
>> >> COMPLETE_WITH_QUERY(Query_for_list_of_subscriptions[0].query);
>> >
>> > These are version-specific queries, so should be passed in their
>> > entirety to COMPLETE_WITH_VERSIONED_QUERY() so that psql can pick the
>> > right version, and avoid sending the query at all if the server is too
>> > old.
>>
>> +1.
>
> Modified
>
> >> +/* add these to Query_for_list_of_roles in OWNER TO contexts */
>> >> +#define Keywords_for_list_of_owner_to_roles \
>> >> +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
>> >
>> > I think this would read better without the TO, both in the comment and
>> > the constant name, similar to the below only having GRANT without TO:
>>
>> Keywords_for_list_of_grant_roles is used in six code paths, so it
>> seems to me that there is little gain in having a separate #define
>> here. Let's just specify the list of allowed roles (RoleSpec) where
>> OWNER TO is parsed.
>
> I have removed the macro and specified the allowed roles.
>
> Thanks for the comments, the attached v2 patch has the changes for the same.
LGTM, +1 to commit.
- ilmari
Re: Miscellaneous tab completion issue fixes
On Tue, 4 Oct 2022 at 09:13, Michael Paquier wrote:
>
> On Mon, Oct 03, 2022 at 06:29:32PM +0100, Dagfinn Ilmari Mannsåker wrote:
> > vignesh C writes:
> >> +else if (TailMatchesCS("\\dRp*"))
> >> +COMPLETE_WITH_QUERY(Query_for_list_of_publications[0].query);
> >> +else if (TailMatchesCS("\\dRs*"))
> >> +COMPLETE_WITH_QUERY(Query_for_list_of_subscriptions[0].query);
> >
> > These are version-specific queries, so should be passed in their
> > entirety to COMPLETE_WITH_VERSIONED_QUERY() so that psql can pick the
> > right version, and avoid sending the query at all if the server is too
> > old.
>
> +1.
>
Modified
>> +/* add these to Query_for_list_of_roles in OWNER TO contexts */
> >> +#define Keywords_for_list_of_owner_to_roles \
> >> +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
> >
> > I think this would read better without the TO, both in the comment and
> > the constant name, similar to the below only having GRANT without TO:
>
> Keywords_for_list_of_grant_roles is used in six code paths, so it
> seems to me that there is little gain in having a separate #define
> here. Let's just specify the list of allowed roles (RoleSpec) where
> OWNER TO is parsed.
I have removed the macro and specified the allowed roles.
Thanks for the comments, the attached v2 patch has the changes for the same.
Regards,
Vignesh
From 525eb4d3959f316338160bfbcc769f64c16310a0 Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Sun, 2 Oct 2022 10:59:59 +0530
Subject: [PATCH v2 2/2] Include CURRENT_ROLE, CURRENT_USER and SESSION_USER in
tab completion while changing owner.
Include CURRENT_ROLE, CURRENT_USER and SESSION_USER in tab completion
while changing owner.
---
src/bin/psql/tab-complete.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2818fb26a0..584d9d5ae6 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4160,7 +4160,10 @@ 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_PLUS(Query_for_list_of_roles,
+ "CURRENT_ROLE",
+ "CURRENT_USER",
+ "SESSION_USER");
/* ORDER BY */
else if (TailMatches("FROM", MatchAny, "ORDER"))
--
2.32.0
From 890ac3d01ecd4238572047e738a80acd1a5c7a67 Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Sun, 2 Oct 2022 10:45:04 +0530
Subject: [PATCH v2 1/2] Display publications and subscriptions for tab
completion of \dRp and \dRs.
Display publications and subscriptions for tab completion of \dRp and
\dRs.
---
src/bin/psql/tab-complete.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 71cfe8aec1..2818fb26a0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4614,6 +4614,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_partitioned_tables);
else if (TailMatchesCS("\\dP*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_partitioned_relations);
+ else if (TailMatchesCS("\\dRp*"))
+ COMPLETE_WITH_VERSIONED_QUERY(Query_for_list_of_publications);
+ else if (TailMatchesCS("\\dRs*"))
+ COMPLETE_WITH_VERSIONED_QUERY(Query_for_list_of_subscriptions);
else if (TailMatchesCS("\\ds*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
else if (TailMatchesCS("\\dt*"))
--
2.32.0
Re: Miscellaneous tab completion issue fixes
On Mon, Oct 03, 2022 at 06:29:32PM +0100, Dagfinn Ilmari Mannsåker wrote:
> vignesh C writes:
>> +else if (TailMatchesCS("\\dRp*"))
>> +COMPLETE_WITH_QUERY(Query_for_list_of_publications[0].query);
>> +else if (TailMatchesCS("\\dRs*"))
>> +COMPLETE_WITH_QUERY(Query_for_list_of_subscriptions[0].query);
>
> These are version-specific queries, so should be passed in their
> entirety to COMPLETE_WITH_VERSIONED_QUERY() so that psql can pick the
> right version, and avoid sending the query at all if the server is too
> old.
+1.
>> +/* add these to Query_for_list_of_roles in OWNER TO contexts */
>> +#define Keywords_for_list_of_owner_to_roles \
>> +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
>
> I think this would read better without the TO, both in the comment and
> the constant name, similar to the below only having GRANT without TO:
Keywords_for_list_of_grant_roles is used in six code paths, so it
seems to me that there is little gain in having a separate #define
here. Let's just specify the list of allowed roles (RoleSpec) where
OWNER TO is parsed.
--
Michael
signature.asc
Description: PGP signature
Re: Miscellaneous tab completion issue fixes
vignesh C writes:
> Hi,
>
> There were a couple of tab completion issues present:
> a) \dRp and \dRs tab completion displays tables instead of displaying
> publications and subscriptions.
> b) "ALTER ... OWNER TO" does not include displaying of CURRENT_ROLE,
> CURRENT_USER and SESSION_USER.
>
> The attached patch has the changes to handle the same.
Good catches! Just a few comments:
> + else if (TailMatchesCS("\\dRp*"))
> + COMPLETE_WITH_QUERY(Query_for_list_of_publications[0].query);
> + else if (TailMatchesCS("\\dRs*"))
> + COMPLETE_WITH_QUERY(Query_for_list_of_subscriptions[0].query);
These are version-specific queries, so should be passed in their
entirety to COMPLETE_WITH_VERSIONED_QUERY() so that psql can pick the
right version, and avoid sending the query at all if the server is too
old.
> +/* add these to Query_for_list_of_roles in OWNER TO contexts */
> +#define Keywords_for_list_of_owner_to_roles \
> +"CURRENT_ROLE", "CURRENT_USER", "SESSION_USER"
I think this would read better without the TO, both in the comment and
the constant name, similar to the below only having GRANT without TO:
> /* 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"
- ilmari
Miscellaneous tab completion issue fixes
Hi,
There were a couple of tab completion issues present:
a) \dRp and \dRs tab completion displays tables instead of displaying
publications and subscriptions.
b) "ALTER ... OWNER TO" does not include displaying of CURRENT_ROLE,
CURRENT_USER and SESSION_USER.
The attached patch has the changes to handle the same.
Regards,
Vignesh
From 1d454be7e89828bdacb191681e469c9581b615d5 Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Sun, 2 Oct 2022 10:45:04 +0530
Subject: [PATCH v1 1/2] Display publications and subscriptions for tab
completion of \dRp and \dRs.
Display publications and subscriptions for tab completion of \dRp and
\dRs.
---
src/bin/psql/tab-complete.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 71cfe8aec1..4cda92208b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4614,6 +4614,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_partitioned_tables);
else if (TailMatchesCS("\\dP*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_partitioned_relations);
+ else if (TailMatchesCS("\\dRp*"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_publications[0].query);
+ else if (TailMatchesCS("\\dRs*"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_subscriptions[0].query);
else if (TailMatchesCS("\\ds*"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
else if (TailMatchesCS("\\dt*"))
--
2.32.0
From 1c16b36c183f610fb79607f86663a8f6d655d70c Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Sun, 2 Oct 2022 10:59:59 +0530
Subject: [PATCH v1 2/2] Include CURRENT_ROLE, CURRENT_USER and SESSION_USER in
tab completion while changing owner.
Include CURRENT_ROLE, CURRENT_USER and SESSION_USER in tab completion
while changing owner.
---
src/bin/psql/tab-complete.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4cda92208b..4e71f4371b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1031,6 +1031,10 @@ 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 TO contexts */
+#define Keywords_for_list_of_owner_to_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"
@@ -4160,7 +4164,8 @@ 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_PLUS(Query_for_list_of_roles,
+ Keywords_for_list_of_owner_to_roles);
/* ORDER BY */
else if (TailMatches("FROM", MatchAny, "ORDER"))
--
2.32.0
