Patch applied.

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

Bruce Momjian wrote:
> 
> 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

-- 
  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

Reply via email to