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

I wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> I haven't found an explanation in this thread why it does always quote 
>> now.  That seems a bit unusual.  Is there a reason for this?  Can we do 
>> without it?

> The core problem we're trying to solve is stated in the thread title:
> ...
> It'd be possible, perhaps, to distinguish between this case and the
> cases in backslash commands, which are okay with omitted quotes
> (for some filenames).  I'm not sure that that would be an improvement
> though.

I realized that there *is* a good reason to worry about this.  Fairly
recent versions of libedit will try to backslash-escape the output
of psql_completion(), as I described over at [1].  This is absolutely
disastrous if we've emitted a quoted filename: we end up going from,
say,
    \lo_import myf<TAB>
to
    \lo_import \'myfile\'
which of course doesn't work at all (psql thinks \' is in itself a
backslash command).

There isn't much we can do about this in contexts where we must
quote the filename: not doing so produces an equally broken command.
However, this problem does suggest that we shouldn't force quoting
if we don't have to, as that just breaks cases we needn't break.

Hence, the attached revision only forces quoting in a SQL COPY
command, or if the user already typed a quote.

I also added some regression tests (whee!).  They pass for me with a
couple different readline and libedit versions, but I have only minimal
hopes for them passing everywhere without further hacking ... the
results of the other thread suggest that pain is to be expected here.

            regards, tom lane

[1] https://www.postgresql.org/message-id/13708.1578059577%40sss.pgh.pa.us

diff --git a/config/programs.m4 b/config/programs.m4
index 90ff944..68ab823 100644
--- a/config/programs.m4
+++ b/config/programs.m4
@@ -209,17 +209,20 @@ fi



-# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
-# ---------------------------------------
+# PGAC_READLINE_VARIABLES
+# -----------------------
 # Readline versions < 2.1 don't have rl_completion_append_character
+# Libedit lacks rl_filename_quote_characters and rl_filename_quoting_function

-AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],
+AC_DEFUN([PGAC_READLINE_VARIABLES],
 [AC_CACHE_CHECK([for rl_completion_append_character], pgac_cv_var_rl_completion_append_character,
 [AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
-#ifdef HAVE_READLINE_READLINE_H
-# include <readline/readline.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
 #elif defined(HAVE_READLINE_H)
-# include <readline.h>
+#include <readline.h>
 #endif
 ],
 [rl_completion_append_character = 'x';])],
@@ -228,7 +231,42 @@ AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],
 if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
 AC_DEFINE(HAVE_RL_COMPLETION_APPEND_CHARACTER, 1,
           [Define to 1 if you have the global variable 'rl_completion_append_character'.])
-fi])# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
+fi
+AC_CACHE_CHECK([for rl_filename_quote_characters], pgac_cv_var_rl_filename_quote_characters,
+[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
+#elif defined(HAVE_READLINE_H)
+#include <readline.h>
+#endif
+],
+[rl_filename_quote_characters = "x";])],
+[pgac_cv_var_rl_filename_quote_characters=yes],
+[pgac_cv_var_rl_filename_quote_characters=no])])
+if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then
+AC_DEFINE(HAVE_RL_FILENAME_QUOTE_CHARACTERS, 1,
+          [Define to 1 if you have the global variable 'rl_filename_quote_characters'.])
+fi
+AC_CACHE_CHECK([for rl_filename_quoting_function], pgac_cv_var_rl_filename_quoting_function,
+[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
+#elif defined(HAVE_READLINE_H)
+#include <readline.h>
+#endif
+],
+[rl_filename_quoting_function = 0;])],
+[pgac_cv_var_rl_filename_quoting_function=yes],
+[pgac_cv_var_rl_filename_quoting_function=no])])
+if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then
+AC_DEFINE(HAVE_RL_FILENAME_QUOTING_FUNCTION, 1,
+          [Define to 1 if you have the global variable 'rl_filename_quoting_function'.])
+fi
+])# PGAC_READLINE_VARIABLES



diff --git a/configure b/configure
index d2d63f2..40fe9c1 100755
--- a/configure
+++ b/configure
@@ -16316,10 +16316,12 @@ else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdio.h>
-#ifdef HAVE_READLINE_READLINE_H
-# include <readline/readline.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
 #elif defined(HAVE_READLINE_H)
-# include <readline.h>
+#include <readline.h>
 #endif

 int
@@ -16345,6 +16347,85 @@ if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
 $as_echo "#define HAVE_RL_COMPLETION_APPEND_CHARACTER 1" >>confdefs.h

 fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quote_characters" >&5
+$as_echo_n "checking for rl_filename_quote_characters... " >&6; }
+if ${pgac_cv_var_rl_filename_quote_characters+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stdio.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
+#elif defined(HAVE_READLINE_H)
+#include <readline.h>
+#endif
+
+int
+main ()
+{
+rl_filename_quote_characters = "x";
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  pgac_cv_var_rl_filename_quote_characters=yes
+else
+  pgac_cv_var_rl_filename_quote_characters=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quote_characters" >&5
+$as_echo "$pgac_cv_var_rl_filename_quote_characters" >&6; }
+if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then
+
+$as_echo "#define HAVE_RL_FILENAME_QUOTE_CHARACTERS 1" >>confdefs.h
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quoting_function" >&5
+$as_echo_n "checking for rl_filename_quoting_function... " >&6; }
+if ${pgac_cv_var_rl_filename_quoting_function+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stdio.h>
+#if defined(HAVE_READLINE_READLINE_H)
+#include <readline/readline.h>
+#elif defined(HAVE_EDITLINE_READLINE_H)
+#include <editline/readline.h>
+#elif defined(HAVE_READLINE_H)
+#include <readline.h>
+#endif
+
+int
+main ()
+{
+rl_filename_quoting_function = 0;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  pgac_cv_var_rl_filename_quoting_function=yes
+else
+  pgac_cv_var_rl_filename_quoting_function=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quoting_function" >&5
+$as_echo "$pgac_cv_var_rl_filename_quoting_function" >&6; }
+if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then
+
+$as_echo "#define HAVE_RL_FILENAME_QUOTING_FUNCTION 1" >>confdefs.h
+
+fi
+
   for ac_func in rl_completion_matches rl_filename_completion_function rl_reset_screen_size
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
diff --git a/configure.in b/configure.in
index 1599fc5..8165f70 100644
--- a/configure.in
+++ b/configure.in
@@ -1874,7 +1874,7 @@ fi
 LIBS="$LIBS_including_readline"

 if test "$with_readline" = yes; then
-  PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
+  PGAC_READLINE_VARIABLES
   AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function rl_reset_screen_size])
   AC_CHECK_FUNCS([append_history history_truncate_file])
 fi
diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.c
index c74e58d..c521749 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 1fb1cfc..4be172e 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/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index ef6c28f..49f6d95 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -50,6 +50,30 @@ delete $ENV{TERM};
 # Some versions of readline inspect LS_COLORS, so for luck unset that too.
 delete $ENV{LS_COLORS};

+# In a VPATH build, we'll be started in the source directory, but we want
+# to run in the build directory so that we can use relative paths to
+# access the tmp_check subdirectory; otherwise the output from filename
+# completion tests is too variable.
+if ($ENV{TESTDIR})
+{
+    chdir $ENV{TESTDIR} or die "could not chdir to \"$ENV{TESTDIR}\": $!";
+}
+
+# Create some junk files for filename completion testing.
+my $FH;
+open $FH, ">", "tmp_check/somefile"
+  or die("could not create file \"tmp_check/somefile\": $!");
+print $FH "some stuff\n";
+close $FH;
+open $FH, ">", "tmp_check/afile123"
+  or die("could not create file \"tmp_check/afile123\": $!");
+print $FH "more stuff\n";
+close $FH;
+open $FH, ">", "tmp_check/afile456"
+  or die("could not create file \"tmp_check/afile456\": $!");
+print $FH "other stuff\n";
+close $FH;
+
 # fire up an interactive psql session
 my $in  = '';
 my $out = '';
@@ -94,6 +118,15 @@ sub clear_query
     return;
 }

+# Clear current line to start over
+# (this will work in an incomplete string literal, but it's less desirable
+# than clear_query because we lose evidence in the history file)
+sub clear_line
+{
+    check_completion("\025\n", qr/postgres=# /, "control-U works");
+    return;
+}
+
 # check basic command completion: SEL<tab> produces SELECT<space>
 check_completion("SEL\t", qr/SELECT /, "complete SEL<tab> to SELECT");

@@ -132,6 +165,47 @@ check_completion("\\DRD\t", qr/drds /, "complete \\DRD<tab> to \\drds");

 clear_query();

+# check filename completion
+check_completion(
+    "\\lo_import tmp_check/some\t",
+    qr|tmp_check/somefile |,
+    "filename completion with one possibility");
+
+clear_query();
+
+# note: readline might print a bell before the completion
+check_completion(
+    "\\lo_import tmp_check/af\t",
+    qr|tmp_check/af\a?ile|,
+    "filename completion with multiple possibilities");
+
+clear_query();
+
+# COPY requires quoting
+# note: broken versions of libedit want to backslash the closing quote;
+# not much we can do about that
+check_completion(
+    "COPY foo FROM tmp_check/some\t",
+    qr|'tmp_check/somefile\\?' |,
+    "quoted filename completion with one possibility");
+
+clear_line();
+
+check_completion(
+    "COPY foo FROM tmp_check/af\t",
+    qr|'tmp_check/afile|,
+    "quoted filename completion with multiple possibilities");
+
+# some versions of readline/libedit require two tabs here, some only need one
+# also, some will offer the whole path name and some just the file name
+# the quotes might appear, too
+check_completion(
+    "\t\t",
+    qr|afile123'? +'?(tmp_check/)?afile456|,
+    "offer multiple file choices");
+
+clear_line();
+
 # send psql an explicit \q to shut it down, else pty won't close properly
 $timer->start(5);
 $in .= "\\q\n";
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 2fd8886..4c80b9a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -41,6 +41,7 @@
 #ifdef USE_READLINE

 #include <ctype.h>
+#include <sys/stat.h>

 #include "catalog/pg_am_d.h"
 #include "catalog/pg_class_d.h"
@@ -63,6 +64,15 @@
 #define rl_completion_matches completion_matches
 #endif

+/*
+ * Currently we assume that rl_filename_dequoting_function exists if
+ * rl_filename_quoting_function does.  If that proves not to be the case,
+ * we'd need to test for the former, or possibly both, in configure.
+ */
+#ifdef HAVE_RL_FILENAME_QUOTING_FUNCTION
+#define USE_FILENAME_QUOTING_FUNCTIONS 1
+#endif
+
 /* word break characters */
 #define WORD_BREAKS        "\t\n@$><=;|&{() "

@@ -157,9 +167,11 @@ typedef struct SchemaQuery
 static int    completion_max_records;

 /*
- * Communication variables set by COMPLETE_WITH_FOO macros and then used by
- * the completion callback functions.  Ugly but there is no better way.
+ * Communication variables set by psql_completion (mostly in COMPLETE_WITH_FOO
+ * macros) and then used by the completion callback functions.  Ugly but there
+ * is no better way.
  */
+static char completion_last_char;    /* last char of input word */
 static const char *completion_charp;    /* to pass a string */
 static const char *const *completion_charpp;    /* to pass a list of strings */
 static const char *completion_info_charp;    /* to pass a second string */
@@ -167,6 +179,7 @@ static const char *completion_info_charp2;    /* to pass a third string */
 static const VersionedQuery *completion_vquery; /* to pass a VersionedQuery */
 static const SchemaQuery *completion_squery;    /* to pass a SchemaQuery */
 static bool completion_case_sensitive;    /* completion is case sensitive */
+static bool completion_force_quote; /* true to force-quote filenames */

 /*
  * A few macros to ease typing. You can use these to complete the given
@@ -1114,9 +1127,9 @@ static char **get_previous_words(int point, char **buffer, int *nwords);

 static char *get_guctype(const char *varname);

-#ifdef NOT_USED
-static char *quote_file_name(char *text, int match_type, char *quote_pointer);
-static char *dequote_file_name(char *text, char quote_char);
+#ifdef USE_FILENAME_QUOTING_FUNCTIONS
+static char *quote_file_name(char *fname, int match_type, char *quote_pointer);
+static char *dequote_file_name(char *fname, int quote_char);
 #endif


@@ -1129,8 +1142,32 @@ initialize_readline(void)
     rl_readline_name = (char *) pset.progname;
     rl_attempted_completion_function = psql_completion;

+#ifdef USE_FILENAME_QUOTING_FUNCTIONS
+    rl_filename_quoting_function = quote_file_name;
+    rl_filename_dequoting_function = dequote_file_name;
+#endif
+
     rl_basic_word_break_characters = WORD_BREAKS;

+    rl_completer_quote_characters = "'\"";
+
+    /*
+     * Set rl_filename_quote_characters to "all possible characters",
+     * otherwise Readline will skip filename quoting if it thinks a filename
+     * doesn't need quoting.  Readline actually interprets this as bytes, so
+     * there are no encoding considerations here.
+     */
+#ifdef HAVE_RL_FILENAME_QUOTE_CHARACTERS
+    {
+        unsigned char *fqc = (unsigned char *) pg_malloc(256);
+
+        for (int i = 0; i < 255; i++)
+            fqc[i] = (unsigned char) (i + 1);
+        fqc[255] = '\0';
+        rl_filename_quote_characters = (const char *) fqc;
+    }
+#endif
+
     completion_max_records = 1000;

     /*
@@ -1455,6 +1492,10 @@ psql_completion(const char *text, int start, int end)
     char       *text_copy = pnstrdup(rl_line_buffer + start, end - start);
     text = text_copy;

+    /* Remember last char of the given input word. */
+    completion_last_char = (end > start) ? text[end - start - 1] : '\0';
+
+    /* We usually want the append character to be a space. */
 #ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
     rl_completion_append_character = ' ';
 #endif
@@ -2265,13 +2306,19 @@ psql_completion(const char *text, int start, int end)
              Matches("COPY", "BINARY", MatchAny))
         COMPLETE_WITH("FROM", "TO");
     /* If we have COPY [BINARY] <sth> FROM|TO, complete with filename */
-    else if (Matches("COPY|\\copy", MatchAny, "FROM|TO") ||
+    else if (Matches("COPY", MatchAny, "FROM|TO") ||
              Matches("COPY", "BINARY", MatchAny, "FROM|TO"))
     {
         completion_charp = "";
+        completion_force_quote = true;    /* COPY requires quoted filename */
+        matches = rl_completion_matches(text, complete_from_files);
+    }
+    else if (Matches("\\copy", MatchAny, "FROM|TO"))
+    {
+        completion_charp = "";
+        completion_force_quote = false;
         matches = rl_completion_matches(text, complete_from_files);
     }
-
     /* Handle COPY [BINARY] <sth> FROM|TO filename */
     else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny) ||
              Matches("COPY", "BINARY", MatchAny, "FROM|TO", MatchAny))
@@ -3821,6 +3868,7 @@ psql_completion(const char *text, int start, int end)
                            "\\s|\\w|\\write|\\lo_import"))
     {
         completion_charp = "\\";
+        completion_force_quote = false;
         matches = rl_completion_matches(text, complete_from_files);
     }

@@ -4387,14 +4435,58 @@ 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.
+ *
+ * Caller must also set completion_force_quote to indicate whether to force
+ * quotes around the result.  (The SQL COPY command requires that.)
  */
 static char *
 complete_from_files(const char *text, int state)
 {
+#ifdef USE_FILENAME_QUOTING_FUNCTIONS
+
+    /*
+     * If we're using a version of Readline that supports filename quoting
+     * hooks, rely on those, and invoke rl_filename_completion_function()
+     * without messing with its arguments.  Readline does stuff internally
+     * that does not work well at all if we try to handle dequoting here.
+     * Instead, Readline will call quote_file_name() and dequote_file_name()
+     * (see below) at appropriate times.
+     *
+     * ... or at least, mostly it will.  There are some paths involving
+     * unmatched file names in which Readline never calls quote_file_name(),
+     * and if left to its own devices it will incorrectly append a quote
+     * anyway.  Set rl_completion_suppress_quote to prevent that.  If we do
+     * get to quote_file_name(), we'll clear this again.  (Yes, this seems
+     * like it's working around Readline bugs.)
+     *
+     * (For now, we assume that rl_completion_suppress_quote exists if the
+     * filename quoting hooks do.)
+     */
+    rl_completion_suppress_quote = 1;
+
+    /* If user typed a quote, force quoting (never remove user's quote) */
+    if (*text == '\'')
+        completion_force_quote = true;
+
+    return rl_filename_completion_function(text, state);
+#else
+
+    /*
+     * Otherwise, we have to do the best we can.
+     */
     static const char *unquoted_text;
     char       *unquoted_match;
     char       *ret = NULL;

+    /* If user typed a quote, force quoting (never remove user's quote) */
+    if (*text == '\'')
+        completion_force_quote = true;
+
     if (state == 0)
     {
         /* Initialization: stash the unquoted input. */
@@ -4411,22 +4503,40 @@ complete_from_files(const char *text, int state)
     unquoted_match = rl_filename_completion_function(unquoted_text, 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.
-         */
+        struct stat statbuf;
+        bool        is_dir = (stat(unquoted_match, &statbuf) == 0 &&
+                              S_ISDIR(statbuf.st_mode) != 0);
+
+        /* Re-quote the result, if needed. */
         ret = quote_if_needed(unquoted_match, " \t\r\n\"`",
-                              '\'', *completion_charp, pset.encoding);
+                              '\'', *completion_charp,
+                              completion_force_quote,
+                              pset.encoding);
         if (ret)
             free(unquoted_match);
         else
             ret = unquoted_match;
+
+        /*
+         * If it's a directory, replace trailing quote with a slash; this is
+         * usually more convenient.  (If we didn't quote, leave this to
+         * libedit.)
+         */
+        if (*ret == '\'' && is_dir)
+        {
+            char       *retend = ret + strlen(ret) - 1;
+
+            Assert(*retend == '\'');
+            *retend = '/';
+            /* Try to prevent libedit from adding a space, too */
+#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
+            rl_completion_append_character = '\0';
+#endif
+        }
     }

     return ret;
+#endif                            /* USE_FILENAME_QUOTING_FUNCTIONS */
 }


@@ -4678,46 +4788,108 @@ get_guctype(const char *varname)
     return guctype;
 }

-#ifdef NOT_USED
+#ifdef USE_FILENAME_QUOTING_FUNCTIONS

 /*
- * Surround a string with single quotes. This works for both SQL and
- * psql internal. Currently disabled because it is reported not to
- * cooperate with certain versions of readline.
+ * Quote a filename according to SQL rules, returning a malloc'd string.
+ * completion_charp must point to escape character or '\0', and
+ * completion_force_quote must be set correctly, as per comments for
+ * complete_from_files().
  */
 static char *
-quote_file_name(char *text, int match_type, char *quote_pointer)
+quote_file_name(char *fname, int match_type, char *quote_pointer)
 {
     char       *s;
-    size_t        length;
+    struct stat statbuf;
+
+    /* Quote if needed. */
+    s = quote_if_needed(fname, " \t\r\n\"`",
+                        '\'', *completion_charp,
+                        completion_force_quote,
+                        pset.encoding);
+    if (!s)
+        s = pg_strdup(fname);

-    (void) quote_pointer;        /* not used */
+    /*
+     * However, some of the time we have to strip the trailing quote from what
+     * we send back.  Never strip the trailing quote if the user already typed
+     * one; otherwise, suppress the trailing quote if we have multiple/no
+     * matches (because we don't want to add a quote if the input is seemingly
+     * unfinished), or if the input was already quoted (because Readline will
+     * do arguably-buggy things otherwise), or if the file does not exist, or
+     * if it's a directory.
+     */
+    if (*s == '\'' &&
+        completion_last_char != '\'' &&
+        (match_type != SINGLE_MATCH ||
+         (quote_pointer && *quote_pointer == '\'') ||
+         stat(fname, &statbuf) != 0 ||
+         S_ISDIR(statbuf.st_mode)))
+    {
+        char       *send = s + strlen(s) - 1;
+
+        Assert(*send == '\'');
+        *send = '\0';
+    }
+
+    /*
+     * And now we can let Readline do its thing with possibly adding a quote
+     * on its own accord.  (This covers some additional cases beyond those
+     * dealt with above.)
+     */
+    rl_completion_suppress_quote = 0;
+
+    /*
+     * If user typed a leading quote character other than single quote (i.e.,
+     * double quote), zap it, so that we replace it with the correct single
+     * quote.
+     */
+    if (quote_pointer && *quote_pointer != '\'')
+        *quote_pointer = '\0';

-    length = strlen(text) +(match_type == SINGLE_MATCH ? 3 : 2);
-    s = pg_malloc(length);
-    s[0] = '\'';
-    strcpy(s + 1, text);
-    if (match_type == SINGLE_MATCH)
-        s[length - 2] = '\'';
-    s[length - 1] = '\0';
     return s;
 }

+/*
+ * Dequote a filename, if it's quoted.
+ * completion_charp must point to escape character or '\0', as per
+ * comments for complete_from_files().
+ */
 static char *
-dequote_file_name(char *text, char quote_char)
+dequote_file_name(char *fname, int quote_char)
 {
-    char       *s;
-    size_t        length;
+    char       *unquoted_fname;

-    if (!quote_char)
-        return pg_strdup(text);
+    /*
+     * If quote_char is set, it's not included in "fname".  We have to add it
+     * or strtokx will not interpret the string correctly (notably, it won't
+     * recognize escapes).
+     */
+    if (quote_char == '\'')
+    {
+        char       *workspace = (char *) pg_malloc(strlen(fname) + 2);

-    length = strlen(text);
-    s = pg_malloc(length - 2 + 1);
-    strlcpy(s, text +1, length - 2 + 1);
+        workspace[0] = quote_char;
+        strcpy(workspace + 1, fname);
+        unquoted_fname = strtokx(workspace, "", NULL, "'", *completion_charp,
+                                 false, true, pset.encoding);
+        free(workspace);
+    }
+    else
+        unquoted_fname = strtokx(fname, "", NULL, "'", *completion_charp,
+                                 false, true, pset.encoding);

-    return s;
+    /* expect a NULL return for the empty string only */
+    if (!unquoted_fname)
+    {
+        Assert(*fname == '\0');
+        unquoted_fname = fname;
+    }
+
+    /* readline expects a malloc'd result that it is to free */
+    return pg_strdup(unquoted_fname);
 }
-#endif                            /* NOT_USED */
+
+#endif                            /* USE_FILENAME_QUOTING_FUNCTIONS */

 #endif                            /* USE_READLINE */
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 050c48b..983d94e 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -488,6 +488,14 @@
 /* Define to 1 if you have the `rl_filename_completion_function' function. */
 #undef HAVE_RL_FILENAME_COMPLETION_FUNCTION

+/* Define to 1 if you have the global variable 'rl_filename_quote_characters'.
+   */
+#undef HAVE_RL_FILENAME_QUOTE_CHARACTERS
+
+/* Define to 1 if you have the global variable 'rl_filename_quoting_function'.
+   */
+#undef HAVE_RL_FILENAME_QUOTING_FUNCTION
+
 /* Define to 1 if you have the `rl_reset_screen_size' function. */
 #undef HAVE_RL_RESET_SCREEN_SIZE

diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 909bded..979964c 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -333,6 +333,8 @@ sub GenerateFiles
         HAVE_RL_COMPLETION_APPEND_CHARACTER      => undef,
         HAVE_RL_COMPLETION_MATCHES               => undef,
         HAVE_RL_FILENAME_COMPLETION_FUNCTION     => undef,
+        HAVE_RL_FILENAME_QUOTE_CHARACTERS        => undef,
+        HAVE_RL_FILENAME_QUOTING_FUNCTION        => undef,
         HAVE_RL_RESET_SCREEN_SIZE                => undef,
         HAVE_SECURITY_PAM_APPL_H                 => undef,
         HAVE_SETPROCTITLE                        => undef,