Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Andres Freund]

On 2015-12-14 12:18:27 +0100, Andres Freund wrote:
> On 2015-12-12 21:04:31 +0900, Michael Paquier wrote:
> > Hi all,
> > 
> > I just bumped into the following thing while looking again at Thomas'
> > patch for psql tab completion:
> > --- a/src/bin/psql/tab-complete.c
> > +++ b/src/bin/psql/tab-complete.c
> > @@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
> >  pg_strcasecmp(prev5_wd, "IN") == 0 &&
> >  pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
> >  pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
> > -pg_strcasecmp(prev4_wd, "BY") == 0)
> > +pg_strcasecmp(prev_wd, "BY") == 0)
> > {
> > COMPLETE_WITH_QUERY(Query_for_list_of_roles);
> > This should be backpatched, attached is the needed patch.
> 
> Hm, this seems to need slightly more expansive surgery.
> 
> Trivially the comments for ALL IN TABLESPACE seem broken/badly copy pasted:
>  /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx */
> the first xxx doesnt make sense.
> 
> Secondly the OWNED BY completion then breaks the SET TABLESPACE
> completion. That's maybe not an outright bug, but seems odd nonetheless.
> 
> Fujii, Stephen?

I'm off to do something else for a while, but attached is where I
stopped at.
>From 0b7fe71b3c27abf97c1a2fdefdd10c1e71d3eba7 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 14 Dec 2015 12:06:59 +0100
Subject: [PATCH] Fix tab completion for ALTER ... TABLESPACE ... OWNED BY.

This was introduced broken, in b2de2a.

Author: Michael Paquier
Discussion: CAB7nPqSHDdSwsJqX0d2XzjqOHr==HdWiubCi4L=zs7yftun...@mail.gmail.com
Backpatch: 9.4, like the commit introducing the bug
---
 src/bin/psql/tab-complete.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8c48881..85f843e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1025,7 +1025,7 @@ psql_completion(const char *text, int start, int end)
 
 		COMPLETE_WITH_LIST(list_ALTER);
 	}
-	/* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx */
+	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */
 	else if (pg_strcasecmp(prev4_wd, "ALL") == 0 &&
 			 pg_strcasecmp(prev3_wd, "IN") == 0 &&
 			 pg_strcasecmp(prev2_wd, "TABLESPACE") == 0)
@@ -1035,15 +1035,24 @@ psql_completion(const char *text, int start, int end)
 
 		COMPLETE_WITH_LIST(list_ALTERALLINTSPC);
 	}
-	/* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx OWNED BY */
+	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY */
 	else if (pg_strcasecmp(prev6_wd, "ALL") == 0 &&
 			 pg_strcasecmp(prev5_wd, "IN") == 0 &&
 			 pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
 			 pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
-			 pg_strcasecmp(prev4_wd, "BY") == 0)
+			 pg_strcasecmp(prev_wd, "BY") == 0)
 	{
 		COMPLETE_WITH_QUERY(Query_for_list_of_roles);
 	}
+	/* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY xxx */
+	else if (pg_strcasecmp(prev7_wd, "ALL") == 0 &&
+			 pg_strcasecmp(prev6_wd, "IN") == 0 &&
+			 pg_strcasecmp(prev5_wd, "TABLESPACE") == 0 &&
+			 pg_strcasecmp(prev3_wd, "OWNED") == 0 &&
+			 pg_strcasecmp(prev2_wd, "BY") == 0)
+	{
+		COMPLETE_WITH_CONST("SET TABLESPACE");
+	}
 	/* ALTER AGGREGATE,FUNCTION  */
 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
 			 (pg_strcasecmp(prev2_wd, "AGGREGATE") == 0 ||
-- 
2.5.0.400.gff86faf.dirty


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Michael Paquier]

On Mon, Dec 14, 2015 at 8:18 PM, Andres Freund  wrote:
> On 2015-12-12 21:04:31 +0900, Michael Paquier wrote:
>> Hi all,
>>
>> I just bumped into the following thing while looking again at Thomas'
>> patch for psql tab completion:
>> --- a/src/bin/psql/tab-complete.c
>> +++ b/src/bin/psql/tab-complete.c
>> @@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
>>  pg_strcasecmp(prev5_wd, "IN") == 0 &&
>>  pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
>>  pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
>> -pg_strcasecmp(prev4_wd, "BY") == 0)
>> +pg_strcasecmp(prev_wd, "BY") == 0)
>> {
>> COMPLETE_WITH_QUERY(Query_for_list_of_roles);
>> This should be backpatched, attached is the needed patch.
>
> Hm, this seems to need slightly more expansive surgery.
>
> Trivially the comments for ALL IN TABLESPACE seem broken/badly copy pasted:
>  /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx */
> the first xxx doesnt make sense.

Indeed.

> Secondly the OWNED BY completion then breaks the SET TABLESPACE
> completion. That's maybe not an outright bug, but seems odd nonetheless.

You mean that this code should do the completion of SET TABLESPACE
after "OWNED BY xxx" has been typed? Are you sure it's worth going
this far?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Andres Freund]

On 2015-12-14 20:58:21 +0900, Michael Paquier wrote:
> On Mon, Dec 14, 2015 at 8:49 PM, Andres Freund  wrote:
> > On 2015-12-14 20:44:20 +0900, Michael Paquier wrote:
> >> + /*
> >> +  * ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED 
> >> BY xxx
> >> +  * SET TABLESPACE.
> >> +  */
> >> + else if (pg_strcasecmp(prev9_wd, "ALL") == 0 &&
> >> +  pg_strcasecmp(prev8_wd, "IN") == 0 &&
> >> +  pg_strcasecmp(prev7_wd, "TABLESPACE") == 0 &&
> >> +  pg_strcasecmp(prev5_wd, "OWNED") == 0 &&
> >> +  pg_strcasecmp(prev4_wd, "BY") == 0 &&
> >> +  pg_strcasecmp(prev2_wd, "SET") == 0 &&
> >> +  pg_strcasecmp(prev_wd, "TABLESPACE") == 0)
> >> + {
> >> + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
> >> + }
> >
> > Isn't that already handled by the normal SET TABLESPACE case?
> 
> No, There is no SET TABLESPACE case, there is a TABLE SET TABLESPACE
> though. Just removing the TABLE seems to be fine..

ALTER TABLE ALL IN TABLESPACE pg_default OWNED BY andres SET TABLESPACE 
works, because of

/*
 * Finally, we look through the list of "things", such as TABLE, INDEX 
and
 * check if that was the previous word. If so, execute the query to get 
a
 * list of them.
 */
else
{
int i;

for (i = 0; words_after_create[i].name; i++)
{
if (pg_strcasecmp(prev_wd, words_after_create[i].name) 
== 0)
{
if (words_after_create[i].query)

COMPLETE_WITH_QUERY(words_after_create[i].query);
else if (words_after_create[i].squery)

COMPLETE_WITH_SCHEMA_QUERY(*words_after_create[i].squery,

   NULL);
break;
}
}
}



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Michael Paquier]

On Mon, Dec 14, 2015 at 9:08 PM, Andres Freund  wrote:
> ALTER TABLE ALL IN TABLESPACE pg_default OWNED BY andres SET TABLESPACE 
> works, because of

Missed that.
-   /* If we have TABLE  SET TABLESPACE provide a list of
tablespaces */
-   else if (pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
-pg_strcasecmp(prev2_wd, "SET") == 0 &&
-pg_strcasecmp(prev_wd, "TABLESPACE") == 0)
-   COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
So you could get rid of that as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Michael Paquier]

On Mon, Dec 14, 2015 at 8:49 PM, Andres Freund  wrote:
> On 2015-12-14 20:44:20 +0900, Michael Paquier wrote:
>> + /*
>> +  * ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY 
>> xxx
>> +  * SET TABLESPACE.
>> +  */
>> + else if (pg_strcasecmp(prev9_wd, "ALL") == 0 &&
>> +  pg_strcasecmp(prev8_wd, "IN") == 0 &&
>> +  pg_strcasecmp(prev7_wd, "TABLESPACE") == 0 &&
>> +  pg_strcasecmp(prev5_wd, "OWNED") == 0 &&
>> +  pg_strcasecmp(prev4_wd, "BY") == 0 &&
>> +  pg_strcasecmp(prev2_wd, "SET") == 0 &&
>> +  pg_strcasecmp(prev_wd, "TABLESPACE") == 0)
>> + {
>> + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
>> + }
>
> Isn't that already handled by the normal SET TABLESPACE case?

No, There is no SET TABLESPACE case, there is a TABLE SET TABLESPACE
though. Just removing the TABLE seems to be fine..
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Michael Paquier]

On Mon, Dec 14, 2015 at 8:36 PM, Andres Freund  wrote:
> On 2015-12-14 12:18:27 +0100, Andres Freund wrote:
>> On 2015-12-12 21:04:31 +0900, Michael Paquier wrote:
>> > Hi all,
>> >
>> > I just bumped into the following thing while looking again at Thomas'
>> > patch for psql tab completion:
>> > --- a/src/bin/psql/tab-complete.c
>> > +++ b/src/bin/psql/tab-complete.c
>> > @@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
>> >  pg_strcasecmp(prev5_wd, "IN") == 0 &&
>> >  pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
>> >  pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
>> > -pg_strcasecmp(prev4_wd, "BY") == 0)
>> > +pg_strcasecmp(prev_wd, "BY") == 0)
>> > {
>> > COMPLETE_WITH_QUERY(Query_for_list_of_roles);
>> > This should be backpatched, attached is the needed patch.
>>
>> Hm, this seems to need slightly more expansive surgery.
>>
>> Trivially the comments for ALL IN TABLESPACE seem broken/badly copy pasted:
>>  /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx */
>> the first xxx doesnt make sense.
>>
>> Secondly the OWNED BY completion then breaks the SET TABLESPACE
>> completion. That's maybe not an outright bug, but seems odd nonetheless.
>>
>> Fujii, Stephen?
>
> I'm off to do something else for a while, but attached is where I
> stopped at.

Just a bit more and you can get the whole set...
-- 
Michael


psql-tab-alltbspace.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Andres Freund]

On 2015-12-12 21:04:31 +0900, Michael Paquier wrote:
> Hi all,
> 
> I just bumped into the following thing while looking again at Thomas'
> patch for psql tab completion:
> --- a/src/bin/psql/tab-complete.c
> +++ b/src/bin/psql/tab-complete.c
> @@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
>  pg_strcasecmp(prev5_wd, "IN") == 0 &&
>  pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
>  pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
> -pg_strcasecmp(prev4_wd, "BY") == 0)
> +pg_strcasecmp(prev_wd, "BY") == 0)
> {
> COMPLETE_WITH_QUERY(Query_for_list_of_roles);
> This should be backpatched, attached is the needed patch.

Hm, this seems to need slightly more expansive surgery.

Trivially the comments for ALL IN TABLESPACE seem broken/badly copy pasted:
 /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx */
the first xxx doesnt make sense.

Secondly the OWNED BY completion then breaks the SET TABLESPACE
completion. That's maybe not an outright bug, but seems odd nonetheless.

Fujii, Stephen?


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Andres Freund]

On 2015-12-14 20:44:20 +0900, Michael Paquier wrote:
> + /*
> +  * ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx OWNED BY 
> xxx
> +  * SET TABLESPACE.
> +  */
> + else if (pg_strcasecmp(prev9_wd, "ALL") == 0 &&
> +  pg_strcasecmp(prev8_wd, "IN") == 0 &&
> +  pg_strcasecmp(prev7_wd, "TABLESPACE") == 0 &&
> +  pg_strcasecmp(prev5_wd, "OWNED") == 0 &&
> +  pg_strcasecmp(prev4_wd, "BY") == 0 &&
> +  pg_strcasecmp(prev2_wd, "SET") == 0 &&
> +  pg_strcasecmp(prev_wd, "TABLESPACE") == 0)
> + {
> + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
> + }

Isn't that already handled by the normal SET TABLESPACE case?


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] psql tab completion bug for ALL IN TABLESPACE

[Michael Paquier]

Hi all,

I just bumped into the following thing while looking again at Thomas'
patch for psql tab completion:
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
 pg_strcasecmp(prev5_wd, "IN") == 0 &&
 pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
 pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
-pg_strcasecmp(prev4_wd, "BY") == 0)
+pg_strcasecmp(prev_wd, "BY") == 0)
{
COMPLETE_WITH_QUERY(Query_for_list_of_roles);
This should be backpatched, attached is the needed patch.

Regards,
-- 
Michael


20151212_psql_alltblspc.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers