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 have found a weird behavior with identifier quoting, which is not the 
>> subject of this patch, but it appears to be affected by it.

> I'll think about how to improve this.  Given that we're dequoting
> filenames explicitly anyway, maybe we don't need to tell readline that
> double-quote is a quoting character.  Another idea is that maybe
> we ought to refactor things so that identifier dequoting and requoting
> is handled explicitly, similarly to the way filenames work now.
> That would make the patch a lot bigger though.

On reflection, it seems like the best bet for the moment is to
remove double-quote from the rl_completer_quote_characters string,
which should restore all behavior around double-quoted strings to
what it was before.  (We have to keep single-quote in that string,
though, or quoted file names fail entirely.)

The only impact this has on filename completion is that we're no
longer bright enough to convert a double-quoted filename to
single-quoted for you, which would be a nice-to-have but it's
hardly core functionality.

At some point it'd be good to undo that and upgrade quoted-identifier
processing, but I don't really want to include such changes in this
patch.  I'm about burned out on tab completion for right now.

            regards, tom lane

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 25cfbcb..a46ba40 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 b35c4f8..c27f216 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -60,6 +60,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 = '';
@@ -104,6 +128,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");

@@ -142,6 +175,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..2871159 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,37 @@ 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;

+    /*
+     * We should include '"' in rl_completer_quote_characters too, but that
+     * will require some upgrades to how we handle quoted identifiers, so
+     * that's for another day.
+     */
+    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 +1497,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 +2311,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 +3873,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 +4440,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 +4508,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 +4793,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);
+
+    /*
+     * 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';
+    }

-    (void) quote_pointer;        /* not used */
+    /*
+     * 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 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);

-    if (!quote_char)
-        return pg_strdup(text);
+        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);

-    length = strlen(text);
-    s = pg_malloc(length - 2 + 1);
-    strlcpy(s, text +1, length - 2 + 1);
+    /* expect a NULL return for the empty string only */
+    if (!unquoted_fname)
+    {
+        Assert(*fname == '\0');
+        unquoted_fname = fname;
+    }

-    return s;
+    /* 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,