Cannot dump/restore text value \N

Manfred Koizar <mkoi-pg@aon.at> writes:

To be clear, this is not about \N as the default external
representation for NULL, I'm talking about a string consisting of the
two characters backslash and uppercase-N.

Now that I look at it, this must have been broken since the beginning of
time, or at least since we made the null representation configurable.
Surprising no one noticed before.

The problem is that the WITH NULL string is compared to the attribute
value *after* debackslashing, and so there is no way to prevent a match
to an actual valid data string.

In older code it seems that the representation of NULL as \N was
hardwired, and this was tested for in the process of debackslashing,
so that the valid data string \\N wouldn't be mistaken for \N.

For the purposes of recognizing the default \N null representation,
it seems we have to compare the null representation string to the
pre-debackslashing input. (This is probably fairly easy to make happen
in CVS tip, but it might be pretty painful in 7.3.) Arguably this is
the right semantics because in the other direction we don't backslash
the outgoing null-representation string. I wonder whether it would
break any existing apps though.

Comments?

regards, tom lane

I have solved my restore problem by editing (the relevant part of) the
dump (:%s/^I\\\\N^I/^I\\\\N ^I/), a one-off solution <g>

Anyway, thanks for your investigation.

On Sun, 05 Oct 2003 19:12:50 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:

it seems we have to compare the null representation string to the
pre-debackslashing input.

Sounds reasonable, IMHO.

I wonder whether it would break any existing apps though.

Couldn't be worse than silently converting valid non-null values to
NULL ...

Servus
Manfred

On Sun, 05 Oct 2003 19:12:50 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:

it seems we have to compare the null representation string to the
pre-debackslashing input.

Here is a patch that does this and adds a few regression tests.

(This is probably fairly easy to make happen
in CVS tip, but it might be pretty painful in 7.3.)

There haven't been too much changes in this area between 7.3 and 7.4.
A patch against 7.3.4 will follow ...

Servus
Manfred

Manfred Koizar <mkoi-pg@aon.at> writes:

Here is a patch that does this and adds a few regression tests.

Uh, I did that already ... for 7.4 at least.

regards, tom lane

On Wed, 08 Oct 2003 11:31:30 +0200, I wrote:

There haven't been too much changes in this area between 7.3 and 7.4.

Here is the patch for 7.3.4 ...

Bruce, I noticed that the original patch submission didn't contain
anything useful as a cvs log message:

Make COPY FROM a bit more compatible with COPY TO regarding
backslashes, especially \N.

Servus
Manfred

Manfred Koizar wrote:

On Wed, 08 Oct 2003 11:31:30 +0200, I wrote:

There haven't been too much changes in this area between 7.3 and 7.4.

Here is the patch for 7.3.4 ...

Bruce, I noticed that the original patch submission didn't contain
anything useful as a cvs log message:

Make COPY FROM a bit more compatible with COPY TO regarding
backslashes, especially \N.

Oh, good point. Thanks.

Can someone explain what was broken? Was it only for non-standard NULL
strings? Would it silently fail? I saw your example and it seemed
strange we had not seen a bug report before.

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

On Wed, 8 Oct 2003 11:33:24 -0400 (EDT), Bruce Momjian
<pgman@candle.pha.pa.us> wrote:

Can someone explain what was broken?

COPY FROM removed backslashes before comparing the input to the
external null representation. (It had a hard-wired special code path
that allowed \N to be recognized.) The text \N was (and still is)
correctly exported as \\N, but \\N was imported as NULL.

Was it only for non-standard NULL strings?

There were problems in both cases.
Standard NULL representation:

fred=# CREATE TABLE a (c1 text, c2 text);
CREATE TABLE
fred=# INSERT INTO a VALUES ('\\N', null);
INSERT 577147 1
fred=# SELECT * FROM a;
c1 | c2
----+----
\N |
(1 row)

fred=# COPY a TO stdout;
\\N \N
fred=# COPY a FROM stdin;
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself.

\\N \N
\.

fred=# SELECT * FROM a;
c1 | c2
----+----
\N |
|
(2 rows)

User defined NULL string:

fred=# CREATE TABLE a (c1 text, c2 text);
CREATE TABLE
fred=# INSERT INTO a VALUES ('\\X', null);
INSERT 577140 1
fred=# SELECT * FROM a;
c1 | c2
----+----
\X |
(1 row)

fred=# COPY a TO stdout WITH NULL AS '\\X';
\\X \X
fred=# COPY a FROM stdin WITH NULL AS '\\X';
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself.

\\X \X
\.

fred=# SELECT * FROM a;
c1 | c2
----+----
\X |
| X
(2 rows)

Would it silently fail?

It would silently insert wrong data, unless a constraint (NOT NULL)
prevented it.

I saw your example and it seemed
strange we had not seen a bug report before.

Because nobody was crazy enough to store \N in his database ...

Tom has already fixed this issue for cvs head. My 7.4 patch wouldn't
apply anyway (built it against Beta 3). You might want to apply the
7.3.4 version, though.

Should I send a new patch with only the regression tests?

Servus
Manfred

Manfred Koizar <mkoi-pg@aon.at> writes:

Should I send a new patch with only the regression tests?

Yeah, if you think they are worth adding.

regards, tom lane

Tom Lane wrote:

Manfred Koizar <mkoi-pg@aon.at> writes:

Should I send a new patch with only the regression tests?

Yeah, if you think they are worth adding.

Regression part of NULL patch 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