From: Tom Lane <tgl@sss.pgh.pa.us>
To: steven.winfield@cantabcapital.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16059: Tab-completion of filenames in COPY commands removes
 required quotes
In-reply-to: <16059-8836946734c02b84@postgresql.org>
References: <16059-8836946734c02b84@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Tue, 15 Oct 2019 13:11:29 -0000"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <30305.1572729234.0@sss.pgh.pa.us>
Date: Sat, 02 Nov 2019 17:14:20 -0400
Message-ID: <30313.1572729260@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <30305.1572729234.1@sss.pgh.pa.us>

PG Bug reporting form <noreply@postgresql.org> writes:
> As per the documentation[1], the COPY command requires the output filename
> to be single-quoted.

> However, when using psql, a partial COPY command such as this...
> COPY pg_catalog.pg_class TO '/usr
> ...will, on hitting TAB, be converted to this...
> COPY pg_catalog.pg_class TO /usr/
> ...requiring the user to move the cursor back to re-insert the single quote
> before finishing the command and executing.

> The issue seems to be somewhere around here[2], where complete_from_files[3]
> is used to suggest replacements - that function strips quotes from the
> existing (partial) filename but doesn't put them back unless quote_if_needed
> is true (which I guess it isn't, unless there is a space in the filename for
> example).

> Note that using the \copy command instead works fine, as filenames do not
> need to be quoted in that case.

Yeah, it seems like a bad idea to override the user's choice to write
a quote, even if one is not formally necessary.  I propose the attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-bug-16059.patch"; charset="us-ascii"
Content-ID: <30305.1572729234.2@sss.pgh.pa.us>
Content-Description: fix-bug-16059.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.c
index 8c8b4c2..23a6f9f 100644
--- a/src/bin/psql/stringutils.c
+++ b/src/bin/psql/stringutils.c
@@ -282,6 +282,7 @@ strip_quotes(char *source, char quote, char escape, in=
t encoding)
  * entails_quote -	any of these present?  need outer quotes
  * quote -			doubled within string, affixed to both ends
  * escape -			doubled within string
+ * force_quote -	if true, quote the output even if it doesn't "need" it
  * encoding -		the active character-set encoding
  *
  * Do not use this as a substitute for PQescapeStringConn().  Use it for
@@ -289,12 +290,13 @@ strip_quotes(char *source, char quote, char escape, =
int encoding)
  */
 char *
 quote_if_needed(const char *source, const char *entails_quote,
-				char quote, char escape, int encoding)
+				char quote, char escape, bool force_quote,
+				int encoding)
 {
 	const char *src;
 	char	   *ret;
 	char	   *dst;
-	bool		need_quotes =3D false;
+	bool		need_quotes =3D force_quote;
 =

 	Assert(source !=3D NULL);
 	Assert(quote !=3D '\0');
diff --git a/src/bin/psql/stringutils.h b/src/bin/psql/stringutils.h
index 16a7af0..fa00141 100644
--- a/src/bin/psql/stringutils.h
+++ b/src/bin/psql/stringutils.h
@@ -22,6 +22,7 @@ extern char *strtokx(const char *s,
 extern void strip_quotes(char *source, char quote, char escape, int encod=
ing);
 =

 extern char *quote_if_needed(const char *source, const char *entails_quot=
e,
-							 char quote, char escape, int encoding);
+							 char quote, char escape, bool force_quote,
+							 int encoding);
 =

 #endif							/* STRINGUTILS_H */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2b1e3cd..3cc1ae4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4345,6 +4345,12 @@ complete_from_variables(const char *text, const cha=
r *prefix, const char *suffix
  * This function wraps rl_filename_completion_function() to strip quotes =
from
  * the input before searching for matches and to quote any matches for wh=
ich
  * the consuming command will require it.
+ *
+ * Caller must set completion_charp to a zero- or one-character string
+ * containing the escape character.  This is necessary since \copy has no
+ * escape character, but every other backslash command recognizes "\" as =
an
+ * escape character.  Since we have only two callers, don't bother provid=
ing
+ * a macro to simplify this.
  */
 static char *
 complete_from_files(const char *text, int state)
@@ -4370,14 +4376,13 @@ complete_from_files(const char *text, int state)
 	if (unquoted_match)
 	{
 		/*
-		 * Caller sets completion_charp to a zero- or one-character string
-		 * containing the escape character.  This is necessary since \copy has
-		 * no escape character, but every other backslash command recognizes
-		 * "\" as an escape character.  Since we have only two callers, don't
-		 * bother providing a macro to simplify this.
+		 * Pass the escape char if any.  Also, if original input started with
+		 * "'", force the output to have that too.
 		 */
 		ret =3D quote_if_needed(unquoted_match, " \t\r\n\"`",
-							  '\'', *completion_charp, pset.encoding);
+							  '\'', *completion_charp,
+							  (*text =3D=3D '\''),
+							  pset.encoding);
 		if (ret)
 			free(unquoted_match);
 		else

------- =_aaaaaaaaaa0--