Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2007-02-05 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian pgman@candle.pha.pa.us writes:
  Attached is a patch that errors for \r and \n in delimiter and
  null.
 
 I am not convinced that this is a bug.  Can you prove that there is no
 use-case for asking COPY to emit data in this style?  Sure, COPY itself
 couldn't read it, but people sometimes feed COPY output to other
 programs...

FYI, Tom, old email from Feb 1, 2006, from your server:

Received: from sss.pgh.pa.us ([EMAIL PROTECTED] [66.207.139.130])
by momjian.us (8.11.6/8.11.6) with ESMTP id l14HX7l21306
for pgman@candle.pha.pa.us; Sun, 4 Feb 2007 12:33:08 -0500 
(EST)
Received: from sss2.sss.pgh.pa.us ([EMAIL PROTECTED] [127.0.0.1])
by sss.pgh.pa.us (8.13.6/8.13.6) with ESMTP id l14HWWoj020066;
Sun, 4 Feb 2007 12:32:32 -0500 (EST)
To: Bruce Momjian pgman@candle.pha.pa.us
cc: David Fetter [EMAIL PROTECTED], Andrew Dunstan [EMAIL 
PROTECTED],
   [EMAIL PROTECTED], PostgreSQL-patches pgsql-patches@postgresql.org
Subject: Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY 
...
In-Reply-To: [EMAIL PROTECTED]
References: [EMAIL PROTECTED]
Comments: In-reply-to Bruce Momjian pgman@candle.pha.pa.us
message dated Wed, 01 Feb 2006 09:10:51 -0500
   

-- 
  Bruce Momjian   [EMAIL PROTECTED]
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2007-02-05 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 FYI, Tom, old email from Feb 1, 2006, from your server:

Wups.  Sorry about that --- somehow I confused that with the current
thread about copy delimiters.  Should have looked harder at the date.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2007-02-04 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes:
 Attached is a patch that errors for \r and \n in delimiter and
 null.

I am not convinced that this is a bug.  Can you prove that there is no
use-case for asking COPY to emit data in this style?  Sure, COPY itself
couldn't read it, but people sometimes feed COPY output to other
programs...

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-02-01 Thread Bruce Momjian
David Fetter wrote:
 On Wed, Feb 01, 2006 at 01:16:08AM -0500, Tom Lane wrote:
  Bruce Momjian pgman@candle.pha.pa.us writes:
   Attached is a patch that errors for \r and \n in delimiter and
   null.  I kept the ERRCODE_FEATURE_NOT_SUPPORTED error code because
   that is what all the other error tests use in the copy code in
   that area.
  
  I'd go with INVALID_PARAMETER_VALUE, I think.  ISTM that
  FEATURE_NOT_SUPPORTED is appropriate for places where we might
  someday support the case the error is rejecting.  For instance the
  error just above your patch is for a multi-character delimiter
  string.  That isn't completely senseless, it's just not implemented.
  But we're not ever going to allow a delimiter setting that conflicts
  with end-of-line, and I don't foresee allowing some other value for
  end-of-line ;-) ... so this check isn't going to be removed someday.
 
 I don't know why you're saying that the EOL character will never be
 changeable.  Other DBs (yes, I know that's not an argument for doing
 this, but please bear with me) let you set the field separator aka
 our DELIMITER and record separator aka our newline (or CRLF, in some
 cases. Oy!).
 
 Anyhow, Bruce's patch still allows backslash as a delimiter, which can
 cause *all* kinds of fun if not disallowed.

OK, updated patch, which disallows backslash as a delimiter, and updated
error return code.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: src/backend/commands/copy.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.257
diff -c -c -r1.257 copy.c
*** src/backend/commands/copy.c 28 Dec 2005 03:25:32 -  1.257
--- src/backend/commands/copy.c 1 Feb 2006 14:05:53 -
***
*** 856,861 
--- 856,880 
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must be a single 
character)));
  
+   /* Disallow end-of-line characters */
+   if (strchr(cstate-delim, '\r') != NULL ||
+   strchr(cstate-delim, '\n') != NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+errmsg(COPY delimiter cannot be newline or 
carriage return)));
+ 
+   if (strchr(cstate-null_print, '\r') != NULL ||
+   strchr(cstate-null_print, '\n') != NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+errmsg(COPY null cannot use newline or 
carriage return)));
+ 
+   /* Disallow backslash in non-CSV mode */
+   if (!cstate-csv_mode  strchr(cstate-delim, '\\') != NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+errmsg(COPY delimiter cannot be backslash)));
+ 
/* Check header */
if (!cstate-csv_mode  cstate-header_line)
ereport(ERROR,

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread Bruce Momjian
 Is  ERRCODE_FEATURE_NOT_SUPPORTED the right errcode? This isn't a
 missing feature; we are performing a sanity check here. We can
 reasonably expect never to support CR, LF or \ as the text
 delimiter.

I guess that depends on whether we ever plan to allow people to set
the output record separator to something other than CR?LF.

 Maybe ERRCODE_INVALID_PARAMETER_VALUE ? Or maybe we need a new one.

Agreed.  Right now it is invalid and there are no plans to support other
values for end-of-line.  I will make the change when the patch is
applied.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread Bruce Momjian

Uh, couldn't the delimiter be a backslash in CVS mode?

+ #define BADCHARS \r\n\\

Also, should we disable DELIMITER and NULL from sharing characters?

---

David Fetter wrote:
 On Sun, Jan 29, 2006 at 10:20:47PM -0500, Neil Conway wrote:
  On Sun, 2006-01-29 at 17:03 -0800, David Fetter wrote:
   Another followup, this time with the comment done right.
  
  +   /* Disallow the forbidden_delimiter strings */
  +   if (strcspn(cstate-delim, BADCHARS) != 1)
  +   elog(ERROR, COPY delimiter cannot be %#02x,
  +*cstate-delim);
  + 
  
  The comment is still wrong: referencing forbidden_delimiter makes
  it sound like there is something named forbidden_delimiter, but
  there is not (at least in the patch as submitted).
  
  The patch should also use ereport rather than elog, because this
  error message might reasonably be encountered by the user.
 
 Patch with BADCHARS attached :)
 
 Cheers,
 D
 -- 
 David Fetter [EMAIL PROTECTED] http://fetter.org/
 phone: +1 415 235 3778
 
 Remember to vote!

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 3: Have you checked our extensive FAQ?
 
http://www.postgresql.org/docs/faq

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread David Fetter
On Tue, Jan 31, 2006 at 08:03:41PM -0500, Bruce Momjian wrote:
 Uh, couldn't the delimiter be a backslash in CVS mode?

I don't think so.  Folks?

Anyhow, if there are different sets, I could do something like:

#define BADCHARS \r\n\\
#define BADCHARS_CSV \r\n

and then check for csv_mode, etc.

   + #define BADCHARS \r\n\\
 
 Also, should we disable DELIMITER and NULL from sharing characters?

That's on about line 916, post-patch:

/* Don't allow the delimiter to appear in the null string. */
if (strchr(cstate-null_print, cstate-delim[0]) != NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg(COPY delimiter must not appear in the NULL 
specification)));

I suppose that a different error code might be The Right Thing™ here.

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread Andrew Dunstan
David Fetter said:
 On Tue, Jan 31, 2006 at 08:03:41PM -0500, Bruce Momjian wrote:
 Uh, couldn't the delimiter be a backslash in CVS mode?

 I don't think so.  Folks?

Using backslash as a delimiter in CSV would be odd, to say the least. As an
escape char it is occasionally used, but not as a delimiter in my
experience. Maybe we should apply the be liberal in what you accept rule,
but I think this would be stretching it.


 Anyhow, if there are different sets, I could do something like:

 #define BADCHARS \r\n\\
 #define BADCHARS_CSV \r\n

 and then check for csv_mode, etc.

  + #define BADCHARS \r\n\\

 Also, should we disable DELIMITER and NULL from sharing characters?

 That's on about line 916, post-patch:

/* Don't allow the delimiter to appear in the null string. */
if (strchr(cstate-null_print, cstate-delim[0]) != NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg(COPY delimiter must not appear in the NULL
specification)));

 I suppose that a different error code might be The Right Thing™ here.


ERRCODE_WHAT WERE_YOU_THINKING ?

cheers

andrew



---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread David Fetter
On Tue, Jan 31, 2006 at 09:50:26PM -0600, Andrew Dunstan wrote:
 David Fetter said:
  On Tue, Jan 31, 2006 at 08:03:41PM -0500, Bruce Momjian wrote:
  Uh, couldn't the delimiter be a backslash in CVS mode?
 
  I don't think so.  Folks?
 
 Using backslash as a delimiter in CSV would be odd, to say the least. As an
 escape char it is occasionally used, but not as a delimiter in my
 experience. Maybe we should apply the be liberal in what you accept rule,
 but I think this would be stretching it.

aolSo do I./aol

  Also, should we disable DELIMITER and NULL from sharing characters?
 
  That's on about line 916, post-patch:
 
 /* Don't allow the delimiter to appear in the null string. */
 if (strchr(cstate-null_print, cstate-delim[0]) != NULL)
 ereport(ERROR,
 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must not appear in the NULL
 specification)));
 
  I suppose that a different error code might be The Right Thing™
  here.
 
 ERRCODE_WHAT WERE_YOU_THINKING ?

That's an excellent candidate, or maybe ERRCODE_INVALID_PARAMETER_VALUE.
My vote is for ERRCODE_D00D_WTF ;)

Maybe we need an error code for mutually incompatible param values.

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!

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


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread Bruce Momjian
 On Tue, Jan 31, 2006 at 09:50:26PM -0600, Andrew Dunstan wrote:
   Also, should we disable DELIMITER and NULL from sharing characters?
  
   That's on about line 916, post-patch:
  
  /* Don't allow the delimiter to appear in the null string. */
  if (strchr(cstate-null_print, cstate-delim[0]) != NULL)
  ereport(ERROR,
  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  errmsg(COPY delimiter must not appear in the NULL
  specification)));
  
   I suppose that a different error code might be The Right Thing???
   here.
  
  ERRCODE_WHAT WERE_YOU_THINKING ?
 
 That's an excellent candidate, or maybe ERRCODE_INVALID_PARAMETER_VALUE.
 My vote is for ERRCODE_D00D_WTF ;)
 
 Maybe we need an error code for mutually incompatible param values.

Attached is a patch that errors for \r and \n in delimiter and null.  I
kept the ERRCODE_FEATURE_NOT_SUPPORTED error code because that is what
all the other error tests use in the copy code in that area.  I did
nothing with backslash.

FYI, David, my email reader is having problems reading your emails
because of this line:

Content-Type: text/plain; charset=iso_8859_1

My understanding is this should be iso-8859-1, with dashes.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073
Index: src/backend/commands/copy.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.257
diff -c -c -r1.257 copy.c
*** src/backend/commands/copy.c 28 Dec 2005 03:25:32 -  1.257
--- src/backend/commands/copy.c 1 Feb 2006 04:21:31 -
***
*** 856,861 
--- 856,874 
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must be a single 
character)));
  
+   /* Disallow end-of-line characters */
+   if (strchr(cstate-delim, '\r') != NULL ||
+   strchr(cstate-delim, '\n') != NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(COPY delimiter cannot be newline or 
carriage return)));
+ 
+   if (strchr(cstate-null_print, '\r') != NULL ||
+   strchr(cstate-null_print, '\n') != NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(COPY null cannot be newline or 
carriage return)));
+ 
/* Check header */
if (!cstate-csv_mode  cstate-header_line)
ereport(ERROR,

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes:
 Attached is a patch that errors for \r and \n in delimiter and null.  I
 kept the ERRCODE_FEATURE_NOT_SUPPORTED error code because that is what
 all the other error tests use in the copy code in that area.

I'd go with INVALID_PARAMETER_VALUE, I think.  ISTM that
FEATURE_NOT_SUPPORTED is appropriate for places where we might someday
support the case the error is rejecting.  For instance the error just
above your patch is for a multi-character delimiter string.  That isn't
completely senseless, it's just not implemented.  But we're not ever
going to allow a delimiter setting that conflicts with end-of-line,
and I don't foresee allowing some other value for end-of-line ;-)
... so this check isn't going to be removed someday.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-31 Thread David Fetter
On Wed, Feb 01, 2006 at 01:16:08AM -0500, Tom Lane wrote:
 Bruce Momjian pgman@candle.pha.pa.us writes:
  Attached is a patch that errors for \r and \n in delimiter and
  null.  I kept the ERRCODE_FEATURE_NOT_SUPPORTED error code because
  that is what all the other error tests use in the copy code in
  that area.
 
 I'd go with INVALID_PARAMETER_VALUE, I think.  ISTM that
 FEATURE_NOT_SUPPORTED is appropriate for places where we might
 someday support the case the error is rejecting.  For instance the
 error just above your patch is for a multi-character delimiter
 string.  That isn't completely senseless, it's just not implemented.
 But we're not ever going to allow a delimiter setting that conflicts
 with end-of-line, and I don't foresee allowing some other value for
 end-of-line ;-) ... so this check isn't going to be removed someday.

I don't know why you're saying that the EOL character will never be
changeable.  Other DBs (yes, I know that's not an argument for doing
this, but please bear with me) let you set the field separator aka
our DELIMITER and record separator aka our newline (or CRLF, in some
cases. Oy!).

Anyhow, Bruce's patch still allows backslash as a delimiter, which can
cause *all* kinds of fun if not disallowed.

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-30 Thread Andrew Dunstan



David Fetter wrote:

 
+ 	/* Disallow BADCHARS characters */

+   if (strcspn(cstate-delim, BADCHARS) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(COPY delimiter cannot be \%#02x\,
+   *cstate-delim)));
+ 






Is  ERRCODE_FEATURE_NOT_SUPPORTED the right errcode? This isn't a 
missing feature; we are performing a sanity check here. We can 
reasonably expect never to support CR, LF or \ as the text delimiter. 
Maybe ERRCODE_INVALID_PARAMETER_VALUE ? Or maybe we need a new one.


Also, I would probably make the format %#.02x so the result would look 
like 0x0d (for a CR).


(I bet David never thought there would so much fuss over a handful of  
lines of code)


cheers

andrew

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-30 Thread David Fetter
On Mon, Jan 30, 2006 at 08:21:34AM -0500, Andrew Dunstan wrote:
 
 
 David Fetter wrote:
 
  
 +/* Disallow BADCHARS characters */
 +if (strcspn(cstate-delim, BADCHARS) != 1)
 +ereport(ERROR,
 +(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 + errmsg(COPY delimiter cannot be \%#02x\,
 +*cstate-delim)));
 + 
 
 Is  ERRCODE_FEATURE_NOT_SUPPORTED the right errcode? This isn't a
 missing feature; we are performing a sanity check here. We can
 reasonably expect never to support CR, LF or \ as the text
 delimiter.

I guess that depends on whether we ever plan to allow people to set
the output record separator to something other than CR?LF.

 Maybe ERRCODE_INVALID_PARAMETER_VALUE ? Or maybe we need a new one.
 
 Also, I would probably make the format %#.02x so the result would
 look like 0x0d (for a CR).
 
 (I bet David never thought there would so much fuss over a handful
 of  lines of code)

Actually, I'm happy to see it's getting QA.  COPY is something that
has Consequences™ if anything goes wrong with it, so I'd rather do
best efforts up front to get it right. :)

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ... TO

2006-01-29 Thread David Fetter
On Sun, Jan 29, 2006 at 09:50:08PM +, David Fetter wrote:
 
 The following bug has been logged online:
 
 Bug reference:  2221
 Logged by:  David Fetter
 Email address:  [EMAIL PROTECTED]
 PostgreSQL version: all
 Operating system:   all
 Description:Bad delimiters allowed in COPY ... TO
 Details: 
 
 This came up while I was testing my pg_dump specify DELIMITER AS and/or
 NULL AS patch.

Folks,

Please pardon the self-followup.  I believe that this patch fixes the
problem in COPY.

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!
Index: src/backend/commands/copy.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.257
diff -c -r1.257 copy.c
*** src/backend/commands/copy.c 28 Dec 2005 03:25:32 -  1.257
--- src/backend/commands/copy.c 30 Jan 2006 00:39:28 -
***
*** 51,56 
--- 51,57 
  
  #define ISOCTAL(c) (((c) = '0')  ((c) = '7'))
  #define OCTVALUE(c) ((c) - '0')
+ #define BADCHARS \r\n\\
  
  /*
   * Represents the different source/dest cases we need to worry about at
***
*** 856,861 
--- 857,867 
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must be a single 
character)));
  
+   /* Disallow the forbidden_delimiter strings from 
src/include/commands/copy.h */
+   if (strcspn(cstate-delim, BADCHARS) != 1)
+   elog(ERROR, COPY delimiter cannot be %#02x,
+*cstate-delim);
+ 
/* Check header */
if (!cstate-csv_mode  cstate-header_line)
ereport(ERROR,

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ... TO

2006-01-29 Thread David Fetter
On Sun, Jan 29, 2006 at 04:41:43PM -0800, David Fetter wrote:
 On Sun, Jan 29, 2006 at 09:50:08PM +, David Fetter wrote:
  
  The following bug has been logged online:
  
  Bug reference:  2221
  Logged by:  David Fetter
  Email address:  [EMAIL PROTECTED]
  PostgreSQL version: all
  Operating system:   all
  Description:Bad delimiters allowed in COPY ... TO
  Details: 
  
  This came up while I was testing my pg_dump specify DELIMITER AS and/or
  NULL AS patch.
 
 Folks,
 
 Please pardon the self-followup.  I believe that this patch fixes
 the problem in COPY.

Another followup, this time with the comment done right.

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!
Index: src/backend/commands/copy.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.257
diff -c -r1.257 copy.c
*** src/backend/commands/copy.c 28 Dec 2005 03:25:32 -  1.257
--- src/backend/commands/copy.c 30 Jan 2006 01:01:20 -
***
*** 51,56 
--- 51,57 
  
  #define ISOCTAL(c) (((c) = '0')  ((c) = '7'))
  #define OCTVALUE(c) ((c) - '0')
+ #define BADCHARS \r\n\\
  
  /*
   * Represents the different source/dest cases we need to worry about at
***
*** 856,861 
--- 857,867 
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must be a single 
character)));
  
+   /* Disallow the forbidden_delimiter strings */
+   if (strcspn(cstate-delim, BADCHARS) != 1)
+   elog(ERROR, COPY delimiter cannot be %#02x,
+*cstate-delim);
+ 
/* Check header */
if (!cstate-csv_mode  cstate-header_line)
ereport(ERROR,

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


Re: [PATCHES] [BUGS] BUG #2221: Bad delimiters allowed in COPY ...

2006-01-29 Thread David Fetter
On Sun, Jan 29, 2006 at 10:20:47PM -0500, Neil Conway wrote:
 On Sun, 2006-01-29 at 17:03 -0800, David Fetter wrote:
  Another followup, this time with the comment done right.
 
 +   /* Disallow the forbidden_delimiter strings */
 +   if (strcspn(cstate-delim, BADCHARS) != 1)
 +   elog(ERROR, COPY delimiter cannot be %#02x,
 +*cstate-delim);
 + 
 
 The comment is still wrong: referencing forbidden_delimiter makes
 it sound like there is something named forbidden_delimiter, but
 there is not (at least in the patch as submitted).
 
 The patch should also use ereport rather than elog, because this
 error message might reasonably be encountered by the user.

Patch with BADCHARS attached :)

Cheers,
D
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 415 235 3778

Remember to vote!
Index: src/backend/commands/copy.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.257
diff -c -r1.257 copy.c
*** src/backend/commands/copy.c 28 Dec 2005 03:25:32 -  1.257
--- src/backend/commands/copy.c 30 Jan 2006 06:44:10 -
***
*** 51,56 
--- 51,57 
  
  #define ISOCTAL(c) (((c) = '0')  ((c) = '7'))
  #define OCTVALUE(c) ((c) - '0')
+ #define BADCHARS \r\n\\
  
  /*
   * Represents the different source/dest cases we need to worry about at
***
*** 856,861 
--- 857,869 
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 errmsg(COPY delimiter must be a single 
character)));
  
+   /* Disallow BADCHARS characters */
+   if (strcspn(cstate-delim, BADCHARS) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg(COPY delimiter cannot be \%#02x\,
+   *cstate-delim)));
+ 
/* Check header */
if (!cstate-csv_mode  cstate-header_line)
ereport(ERROR,

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org