COPY (... tab completion

Started by Andreas Karlssonabout 10 years ago12 messages

andreas@proxel.se

about 10 years ago

1 attachment(s)

Hi,

Here is a patch which adds tab completion for COPY with a query.
Currently there is only completion for COPY with a relation.

Andreas

Attachments:

copy-tab-compleition.patchtext/x-diff; name=copy-tab-compleition.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 98884eb..98023ca 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1857,11 +1857,18 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (TailMatches1("COPY|\\copy") || TailMatches2("COPY", "BINARY"))
+	else if (TailMatches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+								   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, compelete with list of tables */
+	else if (TailMatches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+	/* If we have COPY (, complete it with legal commands */
+	else if (TailMatches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST4("SELECT", "WITH", "UPDATE", "DELETE");
 	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */
 	else if (TailMatches2("COPY|\\copy|BINARY", MatchAny))
 		COMPLETE_WITH_LIST2("FROM", "TO");

Andreas Karlsson

andreas@proxel.se

about 10 years ago

In reply to: Andreas Karlsson (#1)

1 attachment(s)

Re: COPY (... tab completion

Hi,

I have an updated patch which uses Matches() rather than TailMatches().

Andreas

Attachments:

copy-tab-compleition-v2.patchtext/x-diff; name=copy-tab-compleition-v2.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b5117d4..158b913 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1934,12 +1934,19 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (Matches1("COPY|\\copy") || Matches2("COPY", "BINARY"))
+	else if (Matches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+								   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, compelete with list of tables */
+	else if (Matches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
-	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
+	/* If we have COPY (, complete it with legal commands */
+	else if (TailMatches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST4("SELECT", "WITH", "UPDATE", "DELETE");
+	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */
 	else if (Matches2("COPY|\\copy", MatchAny) ||
 			 Matches3("COPY", "BINARY", MatchAny))
 		COMPLETE_WITH_LIST2("FROM", "TO");

Peter Eisentraut

peter_e@gmx.net

about 10 years ago

In reply to: Andreas Karlsson (#2)

Re: COPY (... tab completion

I think this would be a useful addition. A couple of problems:

This change in the comment doesn't make sense to me and doesn't seem to
match the code:

-	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
+	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

The list of commands to allow as the "query" inside the parentheses is
documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
TABLE should also work. Your list doesn't include all of those. So
please adjust that.

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

Peter Eisentraut

peter_e@gmx.net

about 10 years ago

In reply to: Peter Eisentraut (#3)

Re: COPY (... tab completion

On 1/10/16 8:01 PM, Peter Eisentraut wrote:

The list of commands to allow as the "query" inside the parentheses is
documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
TABLE should also work. Your list doesn't include all of those.

To be fair, this is actually a recent new feature.

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

Tom Lane

tgl@sss.pgh.pa.us

about 10 years ago

In reply to: Peter Eisentraut (#3)

Re: COPY (... tab completion

Peter Eisentraut <peter_e@gmx.net> writes:

I think this would be a useful addition. A couple of problems:
This change in the comment doesn't make sense to me and doesn't seem to
match the code:

-	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
+	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

Offhand, that looks like an accidental reversion of a change made in
9b181b0363deb65b15a9feaf3eb74f86707498a9.

regards, tom lane

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

Andreas Karlsson

andreas@proxel.se

almost 10 years ago

In reply to: Peter Eisentraut (#3)

1 attachment(s)

Re: COPY (... tab completion

On 01/11/2016 02:01 AM, Peter Eisentraut wrote:

I think this would be a useful addition. A couple of problems:

Thanks for the review. A new version is attached.

This change in the comment doesn't make sense to me and doesn't seem to
match the code:
-	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
+	/* If we have COPY|BINARY <sth>, complete it with "TO" or "FROM" */

Fixed. As Tom correctly guessed this was the result of a mistake when
rebasing.

The list of commands to allow as the "query" inside the parentheses is
documented to be: SELECT, VALUES, INSERT, UPDATE or DELETE; and actually
TABLE should also work. Your list doesn't include all of those. So
please adjust that.

Fixed. And TABLE works too.

Andreas

Attachments:

copy-tab-compleition-v2.patchtext/x-diff; name=copy-tab-compleition-v2.patchDownload

commit 3b7a808e710e613f81abd0207847a3378ec3192c
Author: Andreas Karlsson <andreas@proxel.se>
Date:   Sat Dec 12 17:38:19 2015 +0100

    Improve COPY completion

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ad8a580..c928ebf 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1934,11 +1934,18 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (Matches1("COPY|\\copy") || Matches2("COPY", "BINARY"))
+	else if (Matches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+								   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, compelete with list of tables */
+	else if (Matches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+	/* If we have COPY (, complete it with legal commands */
+	else if (TailMatches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST6("SELECT", "TABLE", "WITH", "INSERT", "UPDATE", "DELETE");
 	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
 	else if (Matches2("COPY|\\copy", MatchAny) ||
 			 Matches3("COPY", "BINARY", MatchAny))

Andreas Karlsson

andreas@proxel.se

almost 10 years ago

In reply to: Andreas Karlsson (#6)

1 attachment(s)

Re: COPY (... tab completion

On 01/19/2016 01:57 AM, Andreas Karlsson wrote:

Thanks for the review. A new version is attached.

Whops, attached the wrong file.

Andreas

Attachments:

copy-tab-compleition-v3.patchtext/x-diff; name=copy-tab-compleition-v3.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ad8a580..bc80ed0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1934,11 +1934,18 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (Matches1("COPY|\\copy") || Matches2("COPY", "BINARY"))
+	else if (Matches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+								   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, compelete with list of tables */
+	else if (Matches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+	/* If we have COPY (, complete it with legal commands */
+	else if (TailMatches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT", "UPDATE", "DELETE", "WITH");
 	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
 	else if (Matches2("COPY|\\copy", MatchAny) ||
 			 Matches3("COPY", "BINARY", MatchAny))

Michael Paquier

michael.paquier@gmail.com

almost 10 years ago

In reply to: Andreas Karlsson (#7)

Re: COPY (... tab completion

On Tue, Jan 19, 2016 at 10:12 AM, Andreas Karlsson <andreas@proxel.se> wrote:

On 01/19/2016 01:57 AM, Andreas Karlsson wrote:

Thanks for the review. A new version is attached.

Whops, attached the wrong file.

+ /* If we have COPY BINARY, compelete with list of tables */
s/compelete/complete

+    else if (TailMatches2("COPY|\\copy", "("))
+        COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
"UPDATE", "DELETE", "WITH");
This one should be Matches, no?
-- 
Michael

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

Andreas Karlsson

andreas@proxel.se

almost 10 years ago

In reply to: Michael Paquier (#8)

1 attachment(s)

Re: COPY (... tab completion

On 01/19/2016 07:55 AM, Michael Paquier wrote:

+ /* If we have COPY BINARY, compelete with list of tables */
s/compelete/complete

Fixed.

+    else if (TailMatches2("COPY|\\copy", "("))
+        COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
"UPDATE", "DELETE", "WITH");
This one should be Matches, no?

Yep, fixed.

Andreas

Attachments:

copy-tab-compleition-v4.patchtext/x-diff; name=copy-tab-compleition-v4.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5a11c61..72e0255 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1937,11 +1937,18 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (Matches1("COPY|\\copy") || Matches2("COPY", "BINARY"))
+	else if (Matches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+								   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, complete with list of tables */
+	else if (Matches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+	/* If we have COPY (, complete it with legal commands */
+	else if (Matches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT", "UPDATE", "DELETE", "WITH");
 	/* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
 	else if (Matches2("COPY|\\copy", MatchAny) ||
 			 Matches3("COPY", "BINARY", MatchAny))

#10

Michael Paquier

michael.paquier@gmail.com

almost 10 years ago

In reply to: Andreas Karlsson (#9)

Re: COPY (... tab completion

On Tue, Jan 19, 2016 at 8:00 PM, Andreas Karlsson <andreas@proxel.se> wrote:

On 01/19/2016 07:55 AM, Michael Paquier wrote:

+ /* If we have COPY BINARY, compelete with list of tables */
s/compelete/complete

Fixed.
+    else if (TailMatches2("COPY|\\copy", "("))
+        COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
"UPDATE", "DELETE", "WITH");
This one should be Matches, no?
Yep, fixed.

Marked as ready for committer.

This patch makes me wonder: are we going to nuke the grammar "COPY [
BINARY ] table_name" at some point? This was used up to 7.3.
--
Michael

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

#11

Tom Lane

tgl@sss.pgh.pa.us

almost 10 years ago

In reply to: Michael Paquier (#10)

Re: COPY (... tab completion

Michael Paquier <michael.paquier@gmail.com> writes:

This patch makes me wonder: are we going to nuke the grammar "COPY [
BINARY ] table_name" at some point? This was used up to 7.3.

I'm not particularly in a hurry to remove obsolete syntaxes, as long as
they're not blocking forward progress in some way. However, it seems
to me that it would certainly make sense to remove tab-completion support
for long-deprecated syntax.

regards, tom lane

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

#12

Peter Eisentraut

peter_e@gmx.net

almost 10 years ago

In reply to: Andreas Karlsson (#9)

Re: COPY (... tab completion

On 1/19/16 6:00 AM, Andreas Karlsson wrote:

On 01/19/2016 07:55 AM, Michael Paquier wrote:

+ /* If we have COPY BINARY, compelete with list of tables */
s/compelete/complete

Fixed.
+    else if (TailMatches2("COPY|\\copy", "("))
+        COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
"UPDATE", "DELETE", "WITH");
This one should be Matches, no?
Yep, fixed.

Committed v4, thanks.

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