Re: BUG #16059: Tab-completion of filenames in COPY commands removes required quotes - Mailing list pgsql-bugs

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

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, int 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 = false;
+    bool        need_quotes = force_quote;

     Assert(source != NULL);
     Assert(quote != '\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 encoding);

 extern 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);

 #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 char *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 which
  * 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 providing
+ * 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 = quote_if_needed(unquoted_match, " \t\r\n\"`",
-                              '\'', *completion_charp, pset.encoding);
+                              '\'', *completion_charp,
+                              (*text == '\''),
+                              pset.encoding);
         if (ret)
             free(unquoted_match);
         else