Skip site navigation (1) Skip section navigation (2)

Re: [HACKERS] psql \copy warning

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jeremy Drake <pgsql(at)jdrake(dot)com>
Subject: Re: [HACKERS] psql \copy warning
Date: 2006-05-31 11:02:35
Message-ID: 200605311102.k4VB2Zi21264@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
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(at)candle(dot)pha(dot)pa(dot)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. +

In response to

Responses

pgsql-hackers by date

Next:From: Martijn van OosterhoutDate: 2006-05-31 11:04:11
Subject: Re: [PATCH] Magic block for modules
Previous:From: Marko KreenDate: 2006-05-31 10:08:41
Subject: Re: [PATCH] Magic block for modules

pgsql-patches by date

Next:From: Martijn van OosterhoutDate: 2006-05-31 11:04:11
Subject: Re: [PATCH] Magic block for modules
Previous:From: Marko KreenDate: 2006-05-31 10:08:41
Subject: Re: [PATCH] Magic block for modules

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group