I have developed an updated patch that:

        o  turns off escape_string_warning in pg_dumpall.c
        o  optionally use E'' for \password (undocumented option?)
        o  honor standard_conforming-strings for \copy (but not
           support literal E'' strings)
        o  optionally use E'' for \d commands
        o  turn off escape_string_warning for createdb, createuser,
           droplang

I agree someday we might want to turn off escape_string_warning, but I
think we should leave it on as long as possible as it is still pointing
out escape problem areas in the code.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Right.  I think the question is whether we want all psql strings to
> > > accept backslashes, and hence not support E'' at all for psql commands.
> > > I figured that made the most sense.
> > 
> > I'm not convinced.  Wouldn't it be better if psql commands track the
> > backend syntax?  With standard_conforming_strings on, there will be two
> > ways to tell COPY you want a tab as a delimiter:
> >     DELIMITER '<actual tab char>'
> >     DELIMITER E'\t'
> > and in particular this will NOT do that:
> >     DELIMITER '\t'
> 
> Well, I think it a little more confusing that just \copy.  What about \d
> and \set uses of backslashes.  Do they honor standard_conforming_strings
> too?  I assume you are saying they should.
> 
> > If we keep '\t' as meaning tab in the \copy syntax then I think we're
> > going to cause confusion in the long run.  I think we should fix \copy
> > and related psql backslash commands to accept E'\t', and make sure that
> > the behavior is the same as the connected backend depending on what its 
> > standard_conforming_strings setting is.
> 
> OK, though this is going to mean that examples in the psql manual page
> are going to be different for different standard_conforming_strings
> settings:
> 
>        testdb=> \set content '\'' `cat my_file.txt` '\''
>        testdb=> INSERT INTO my_table VALUES (:content);
> 
> psql doesn't know '''' is about doubling single quotes in a string,
> though \copy does.  The major problem, I think, is that psql often
> follows the shell rules, rather than the SQL rules for most things.
> 
> > There is a secondary, largely cosmetic question of whether psql should
> > attempt to prevent you from seeing escape_string_warning messages.
> > I personally have come to the conclusion that escape_string_warning is
> > probably not going to be on by default anyway ;-), and hence it's not
> > worth going to great extremes to prevent this, particularly if it breaks
> > the ability to use psql against pre-8.1 servers.
> 
> It does break backward compatibility.
> 
> -- 
>   Bruce Momjian   http://candle.pha.pa.us
>   EnterpriseDB    http://www.enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
> 
>                http://archives.postgresql.org
> 

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.77
diff -c -c -r1.77 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c        28 May 2006 21:13:54 -0000      1.77
--- src/bin/pg_dump/pg_dumpall.c        29 May 2006 20:41:01 -0000
***************
*** 338,343 ****
--- 338,345 ----
                printf("SET client_encoding = '%s';\n",
                           pg_encoding_to_char(encoding));
                printf("SET standard_conforming_strings = %s;\n", std_strings);
+               if (strcmp(std_strings, "off") == 0)
+                       printf("SET escape_string_warning = 'off';\n");
                printf("\n");
  
                /* Dump roles (users) */
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.166
diff -c -c -r1.166 command.c
*** src/bin/psql/command.c      2 Apr 2006 20:08:22 -0000       1.166
--- src/bin/psql/command.c      29 May 2006 20:41:02 -0000
***************
*** 681,688 ****
                                PGresult   *res;
  
                                initPQExpBuffer(&buf);
!                               printfPQExpBuffer(&buf, "ALTER USER %s PASSWORD 
'%s';",
!                                                                 fmtId(user), 
encrypted_password);
                                res = PSQLexec(buf.data, false);
                                termPQExpBuffer(&buf);
                                if (!res)
--- 681,689 ----
                                PGresult   *res;
  
                                initPQExpBuffer(&buf);
!                               printfPQExpBuffer(&buf, "ALTER USER %s PASSWORD 
%c'%s';",
!                                                                 fmtId(user), 
NEED_E_STR(encrypted_password),
!                                                                 
encrypted_password);
                                res = PSQLexec(buf.data, false);
                                termPQExpBuffer(&buf);
                                if (!res)
Index: src/bin/psql/common.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/common.h,v
retrieving revision 1.47
diff -c -c -r1.47 common.h
*** src/bin/psql/common.h       6 Mar 2006 19:49:20 -0000       1.47
--- src/bin/psql/common.h       29 May 2006 20:41:03 -0000
***************
*** 23,28 ****
--- 23,34 ----
  #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
  
  /*
+  *    We use this to prefix strings with E'' that we know are already safe,
+  *    so we don't get an escape_string_warning.
+  */
+ #define       NEED_E_STR(str)         ((strchr(str, '\\') && 
!standard_strings()) ? ESCAPE_STRING_SYNTAX : ' ')
+ 
+ /*
   * Safer versions of some standard C library functions. If an
   * out-of-memory condition occurs, these functions will bail out
   * safely; therefore, their return value is guaranteed to be non-NULL.
Index: src/bin/psql/copy.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/copy.c,v
retrieving revision 1.61
diff -c -c -r1.61 copy.c
*** src/bin/psql/copy.c 26 May 2006 19:51:29 -0000      1.61
--- src/bin/psql/copy.c 29 May 2006 20:41:03 -0000
***************
*** 216,222 ****
                goto error;
  
        token = strtokx(NULL, whitespace, NULL, "'",
!                                       '\\', true, pset.encoding);
        if (!token)
                goto error;
  
--- 216,222 ----
                goto error;
  
        token = strtokx(NULL, whitespace, NULL, "'",
!                                       standard_strings() ? 0 : '\\', true, 
pset.encoding);
        if (!token)
                goto error;
  
***************
*** 255,261 ****
        if (token && pg_strcasecmp(token, "delimiters") == 0)
        {
                token = strtokx(NULL, whitespace, NULL, "'",
!                                               '\\', false, pset.encoding);
                if (!token)
                        goto error;
                result->delim = pg_strdup(token);
--- 255,261 ----
        if (token && pg_strcasecmp(token, "delimiters") == 0)
        {
                token = strtokx(NULL, whitespace, NULL, "'",
!                                               standard_strings() ? 0 : '\\', 
false, pset.encoding);
                if (!token)
                        goto error;
                result->delim = pg_strdup(token);
***************
*** 290,299 ****
                        else if (pg_strcasecmp(token, "delimiter") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               '\\', false, 
pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       '\\', 
false, pset.encoding);
                                if (token)
                                        result->delim = pg_strdup(token);
                                else
--- 290,299 ----
                        else if (pg_strcasecmp(token, "delimiter") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token)
                                        result->delim = pg_strdup(token);
                                else
***************
*** 302,311 ****
                        else if (pg_strcasecmp(token, "null") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               '\\', false, 
pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       '\\', 
false, pset.encoding);
                                if (token)
                                        result->null = pg_strdup(token);
                                else
--- 302,311 ----
                        else if (pg_strcasecmp(token, "null") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token)
                                        result->null = pg_strdup(token);
                                else
***************
*** 314,323 ****
                        else if (pg_strcasecmp(token, "quote") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               '\\', false, 
pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       '\\', 
false, pset.encoding);
                                if (token)
                                        result->quote = pg_strdup(token);
                                else
--- 314,323 ----
                        else if (pg_strcasecmp(token, "quote") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token)
                                        result->quote = pg_strdup(token);
                                else
***************
*** 326,335 ****
                        else if (pg_strcasecmp(token, "escape") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               '\\', false, 
pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       '\\', 
false, pset.encoding);
                                if (token)
                                        result->escape = pg_strdup(token);
                                else
--- 326,335 ----
                        else if (pg_strcasecmp(token, "escape") == 0)
                        {
                                token = strtokx(NULL, whitespace, NULL, "'",
!                                                               
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token && pg_strcasecmp(token, "as") == 0)
                                        token = strtokx(NULL, whitespace, NULL, 
"'",
!                                                                       
standard_strings() ? 0 : '\\', false, pset.encoding);
                                if (token)
                                        result->escape = pg_strdup(token);
                                else
***************
*** 462,481 ****
        if (options->delim)
        {
                if (options->delim[0] == '\'')
!                       appendPQExpBuffer(&query, " USING DELIMITERS %s",
!                                                         options->delim);
                else
!                       appendPQExpBuffer(&query, " USING DELIMITERS '%s'",
!                                                         options->delim);
        }
  
        /* There is no backward-compatible CSV syntax */
        if (options->null)
        {
                if (options->null[0] == '\'')
!                       appendPQExpBuffer(&query, " WITH NULL AS %s", 
options->null);
                else
!                       appendPQExpBuffer(&query, " WITH NULL AS '%s'", 
options->null);
        }
  
        if (options->csv_mode)
--- 462,483 ----
        if (options->delim)
        {
                if (options->delim[0] == '\'')
!                       appendPQExpBuffer(&query, " USING DELIMITERS %c%s",
!                                                         
NEED_E_STR(options->delim), options->delim);
                else
!                       appendPQExpBuffer(&query, " USING DELIMITERS %c'%s'",
!                                                         
NEED_E_STR(options->delim), options->delim);
        }
  
        /* There is no backward-compatible CSV syntax */
        if (options->null)
        {
                if (options->null[0] == '\'')
!                       appendPQExpBuffer(&query, " WITH NULL AS %c%s",
!                                                         
NEED_E_STR(options->null), options->null);
                else
!                       appendPQExpBuffer(&query, " WITH NULL AS %c'%s'",
!                                                         
NEED_E_STR(options->null), options->null);
        }
  
        if (options->csv_mode)
***************
*** 487,503 ****
        if (options->quote)
        {
                if (options->quote[0] == '\'')
!                       appendPQExpBuffer(&query, " QUOTE AS %s", 
options->quote);
                else
!                       appendPQExpBuffer(&query, " QUOTE AS '%s'", 
options->quote);
        }
  
        if (options->escape)
        {
                if (options->escape[0] == '\'')
!                       appendPQExpBuffer(&query, " ESCAPE AS %s", 
options->escape);
                else
!                       appendPQExpBuffer(&query, " ESCAPE AS '%s'", 
options->escape);
        }
  
        if (options->force_quote_list)
--- 489,509 ----
        if (options->quote)
        {
                if (options->quote[0] == '\'')
!                       appendPQExpBuffer(&query, " QUOTE AS %c%s",
!                                                         
NEED_E_STR(options->quote), options->quote);
                else
!                       appendPQExpBuffer(&query, " QUOTE AS %c'%s'",
!                                                         
NEED_E_STR(options->quote), options->quote);
        }
  
        if (options->escape)
        {
                if (options->escape[0] == '\'')
!                       appendPQExpBuffer(&query, " ESCAPE AS %c%s",
!                                                         
NEED_E_STR(options->escape), options->escape);
                else
!                       appendPQExpBuffer(&query, " ESCAPE AS %c'%s'",
!                                                         
NEED_E_STR(options->escape), options->escape);
        }
  
        if (options->force_quote_list)
Index: src/bin/psql/describe.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.137
diff -c -c -r1.137 describe.c
*** src/bin/psql/describe.c     28 May 2006 21:13:54 -0000      1.137
--- src/bin/psql/describe.c     29 May 2006 20:41:04 -0000
***************
*** 1907,1920 ****
--- 1907,1923 ----
                        if (altnamevar)
                        {
                                appendPQExpBuffer(buf, "(%s ~ ", namevar);
+                               appendPQExpBufferChar(buf, 
NEED_E_STR(namebuf.data));
                                appendStringLiteralConn(buf, namebuf.data, 
pset.db);
                                appendPQExpBuffer(buf, "\n        OR %s ~ ", 
altnamevar);
+                               appendPQExpBufferChar(buf, 
NEED_E_STR(namebuf.data));
                                appendStringLiteralConn(buf, namebuf.data, 
pset.db);
                                appendPQExpBuffer(buf, ")\n");
                        }
                        else
                        {
                                appendPQExpBuffer(buf, "%s ~ ", namevar);
+                               appendPQExpBufferChar(buf, 
NEED_E_STR(namebuf.data));
                                appendStringLiteralConn(buf, namebuf.data, 
pset.db);
                                appendPQExpBufferChar(buf, '\n');
                        }
***************
*** 1938,1943 ****
--- 1941,1947 ----
                {
                        WHEREAND();
                        appendPQExpBuffer(buf, "%s ~ ", schemavar);
+                       appendPQExpBufferChar(buf, NEED_E_STR(schemabuf.data));
                        appendStringLiteralConn(buf, schemabuf.data, pset.db);
                        appendPQExpBufferChar(buf, '\n');
                }
Index: src/bin/scripts/createdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/scripts/createdb.c,v
retrieving revision 1.19
diff -c -c -r1.19 createdb.c
*** src/bin/scripts/createdb.c  29 May 2006 19:52:46 -0000      1.19
--- src/bin/scripts/createdb.c  29 May 2006 20:41:08 -0000
***************
*** 185,190 ****
--- 185,192 ----
        {
                conn = connectDatabase(dbname, host, port, username, password, 
progname);
  
+               executeCommand(conn, "SET escape_string_warning TO 'off'", 
progname, false);
+ 
                printfPQExpBuffer(&sql, "COMMENT ON DATABASE %s IS ", 
fmtId(dbname));
                appendStringLiteralConn(&sql, comment, conn);
                appendPQExpBuffer(&sql, ";\n");
Index: src/bin/scripts/createuser.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/scripts/createuser.c,v
retrieving revision 1.30
diff -c -c -r1.30 createuser.c
*** src/bin/scripts/createuser.c        29 May 2006 19:52:46 -0000      1.30
--- src/bin/scripts/createuser.c        29 May 2006 20:41:08 -0000
***************
*** 243,248 ****
--- 243,250 ----
        printfPQExpBuffer(&sql, "CREATE ROLE %s", fmtId(newuser));
        if (newpassword)
        {
+               executeCommand(conn, "SET escape_string_warning TO 'off'", 
progname, false);
+ 
                if (encrypted == TRI_YES)
                        appendPQExpBuffer(&sql, " ENCRYPTED");
                if (encrypted == TRI_NO)
Index: src/bin/scripts/droplang.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/scripts/droplang.c,v
retrieving revision 1.20
diff -c -c -r1.20 droplang.c
*** src/bin/scripts/droplang.c  29 May 2006 19:52:46 -0000      1.20
--- src/bin/scripts/droplang.c  29 May 2006 20:41:09 -0000
***************
*** 176,183 ****
         * Force schema search path to be just pg_catalog, so that we don't have
         * to be paranoid about search paths below.
         */
!       executeCommand(conn, "SET search_path = pg_catalog;",
!                                  progname, echo);
  
        /*
         * Make sure the language is installed and find the OIDs of the handler
--- 176,182 ----
         * Force schema search path to be just pg_catalog, so that we don't have
         * to be paranoid about search paths below.
         */
!       executeCommand(conn, "SET search_path = pg_catalog;", progname, echo);
  
        /*
         * Make sure the language is installed and find the OIDs of the handler
---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
       choose an index scan if your joining column's datatypes do not
       match

Reply via email to