Thread: Cannot dump/restore text value \N

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.

CREATE TABLE nonu (tx text NOT NULL);
INSERT INTO nonu VALUES ('\\N');
SELECT * FROM nonu;
COPY nonu TO stdout;

This correctly gives:
\\N

Now try to feed that back into the table:
DELETE FROM nonu;
COPY nonu FROM stdin;
\\N
\.

ERROR:  copy: line 1, CopyFrom: Fail to add null value in not null
attribute tx
lost synchronization with server, resetting connection

This happened with 7.3.4, while trying to restore a 1.3 GB dump :-(
ERROR:  copy: line 809051, CopyFrom: Fail to add null value in not
null attribute text
FATAL:  Socket command type 0 unknown

The bug is still in 7.4Beta3; didn't test with Beta 4 yet.

ServusManfred

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

ServusManfred

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
diff -ruN ../base/src/backend/commands/copy.c src/backend/commands/copy.c
--- ../base/src/backend/commands/copy.c    2003-08-28 15:52:34.000000000 +0200
+++ src/backend/commands/copy.c    2003-10-08 10:43:02.000000000 +0200
@@ -90,7 +90,8 @@
        char *delim, char *null_print);
 static void CopyFrom(Relation rel, List *attnumlist, bool binary, bool oids,
          char *delim, char *null_print);
-static char *CopyReadAttribute(const char *delim, CopyReadResult *result);
+static char *CopyReadAttribute(const char *delim, const char *nullst,
+                               CopyReadResult *result, bool *isnull);
 static Datum CopyReadBinaryAttribute(int column_no, FmgrInfo *flinfo,
                         Oid typelem, bool *isnull);
 static void CopyAttributeOut(char *string, char *delim);
@@ -1361,7 +1362,7 @@

             if (file_has_oids)
             {
-                string = CopyReadAttribute(delim, &result);
+                string = CopyReadAttribute(delim, null_print, &result, &isnull);

                 if (result == END_OF_FILE && *string == '\0')
                 {
@@ -1370,7 +1371,7 @@
                     break;
                 }

-                if (strcmp(string, null_print) == 0)
+                if (isnull)
                     ereport(ERROR,
                             (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
                              errmsg("null OID in COPY data")));
@@ -1403,7 +1404,7 @@
                              errmsg("missing data for column \"%s\"",
                                     NameStr(attr[m]->attname))));

-                string = CopyReadAttribute(delim, &result);
+                string = CopyReadAttribute(delim, null_print, &result, &isnull);

                 if (result == END_OF_FILE && *string == '\0' &&
                     cur == attnumlist && !file_has_oids)
@@ -1413,7 +1414,7 @@
                     break;        /* out of per-attr loop */
                 }

-                if (strcmp(string, null_print) == 0)
+                if (isnull)
                 {
                     /* we read an SQL NULL, no need to do anything */
                 }
@@ -1442,7 +1443,7 @@
             {
                 if (attnumlist == NIL && !file_has_oids)
                 {
-                    string = CopyReadAttribute(delim, &result);
+                    string = CopyReadAttribute(delim, null_print, &result, &isnull);
                     if (result == NORMAL_ATTR || *string != '\0')
                         ereport(ERROR,
                                 (errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
@@ -1650,14 +1651,13 @@
  *        END_OF_FILE:    EOF indicator
  * In all cases, the string read up to the terminator is returned.
  *
- * Note: This function does not care about SQL NULL values -- it
- * is the caller's responsibility to check if the returned string
- * matches what the user specified for the SQL NULL value.
- *
  * delim is the column delimiter string.
+ * nullst says how NULL values are represented.
+ * *isnull is set true if a null attribute, else false.
  */
 static char *
-CopyReadAttribute(const char *delim, CopyReadResult *result)
+CopyReadAttribute(const char *delim, const char *nullst,
+                  CopyReadResult *result, bool *isnull)
 {
     int            c;
     int            delimc = (unsigned char) delim[0];
@@ -1665,6 +1665,17 @@
     unsigned char s[2];
     char       *cvt;
     int            j;
+    bool        matchnull = true;
+    int            matchlen = 0;
+
+#define CHECK_MATCH(c) \
+    do { \
+        if (matchnull) \
+            if (c == nullst[matchlen]) \
+                ++matchlen; \
+            else \
+                matchnull = false; \
+    } while (0)

     s[1] = 0;

@@ -1733,6 +1744,7 @@
         }
         if (c == delimc)
             break;
+        CHECK_MATCH(c);
         if (c == '\\')
         {
             c = CopyGetChar();
@@ -1741,6 +1753,7 @@
                 *result = END_OF_FILE;
                 goto copy_eof;
             }
+            CHECK_MATCH(c);
             switch (c)
             {
                 case '0':
@@ -1760,11 +1773,13 @@
                         {
                             val = (val << 3) + OCTVALUE(c);
                             CopyDonePeek(c, true /* pick up */ );
+                            CHECK_MATCH(c);
                             c = CopyPeekChar();
                             if (ISOCTAL(c))
                             {
                                 val = (val << 3) + OCTVALUE(c);
                                 CopyDonePeek(c, true /* pick up */ );
+                                CHECK_MATCH(c);
                             }
                             else
                             {
@@ -1789,15 +1804,6 @@
                     }
                     break;

-                    /*
-                     * This is a special hack to parse `\N' as
-                     * <backslash-N> rather then just 'N' to provide
-                     * compatibility with the default NULL output. -- pe
-                     */
-                case 'N':
-                    appendStringInfoCharMacro(&attribute_buf, '\\');
-                    c = 'N';
-                    break;
                 case 'b':
                     c = '\b';
                     break;
@@ -1871,6 +1877,7 @@
                     *result = END_OF_FILE;
                     goto copy_eof;
                 }
+                CHECK_MATCH(c);
                 appendStringInfoCharMacro(&attribute_buf, c);
             }
         }
@@ -1878,6 +1885,8 @@

 copy_eof:

+    *isnull = (matchnull && (nullst[matchlen] == '\0'));
+
     if (client_encoding != server_encoding)
     {
         cvt = (char *) pg_client_to_server((unsigned char *) attribute_buf.data,
diff -ruN ../base/src/test/regress/expected/copy2.out src/test/regress/expected/copy2.out
--- ../base/src/test/regress/expected/copy2.out    2003-07-27 06:53:11.000000000 +0200
+++ src/test/regress/expected/copy2.out    2003-10-07 21:12:16.000000000 +0200
@@ -2,7 +2,7 @@
     a serial,
     b int,
     c text not null default 'stuff',
-    d text not null,
+    d text,
     e text
 );
 NOTICE:  CREATE TABLE will create implicit sequence "x_a_seq" for SERIAL column "x.a"
@@ -48,23 +48,38 @@
 CONTEXT:  COPY FROM, line 1
 -- various COPY options: delimiters, oids, NULL string
 COPY x (b, c, d, e) from stdin with oids delimiter ',' null 'x';
+COPY x from stdin WITH DELIMITER AS ';' NULL AS '';
+COPY x from stdin WITH DELIMITER AS ':' NULL AS '\\X';
 -- check results of copy in
 SELECT * FROM x;
-   a   | b  |   c   |   d    |          e
--------+----+-------+--------+----------------------
- 10000 | 21 | 31    | 41     | before trigger fired
- 10001 | 22 | 32    | 42     | before trigger fired
- 10002 | 23 | 33    | 43     | before trigger fired
- 10003 | 24 | 34    | 44     | before trigger fired
- 10004 | 25 | 35    | 45     | before trigger fired
- 10005 | 26 | 36    | 46     | before trigger fired
-     6 |    | 45    | 80     | before trigger fired
-     1 |  1 | stuff | test_1 | after trigger fired
-     2 |  2 | stuff | test_2 | after trigger fired
-     3 |  3 | stuff | test_3 | after trigger fired
-     4 |  4 | stuff | test_4 | after trigger fired
-     5 |  5 | stuff | test_5 | after trigger fired
-(12 rows)
+   a   | b  |     c      |   d    |          e
+-------+----+------------+--------+----------------------
+  9999 |    | \N         | NN     | before trigger fired
+ 10000 | 21 | 31         | 41     | before trigger fired
+ 10001 | 22 | 32         | 42     | before trigger fired
+ 10002 | 23 | 33         | 43     | before trigger fired
+ 10003 | 24 | 34         | 44     | before trigger fired
+ 10004 | 25 | 35         | 45     | before trigger fired
+ 10005 | 26 | 36         | 46     | before trigger fired
+     6 |    | 45         | 80     | before trigger fired
+     7 |    | x          | \x     | before trigger fired
+     8 |    | ,          | \,     | before trigger fired
+  3000 |    | c          |        | before trigger fired
+  4000 |    | C          |        | before trigger fired
+  4001 |  1 | empty      |        | before trigger fired
+  4002 |  2 | null       |        | before trigger fired
+  4003 |  3 | Backslash  | \      | before trigger fired
+  4004 |  4 | BackslashX | \X     | before trigger fired
+  4005 |  5 | N          | N      | before trigger fired
+  4006 |  6 | BackslashN | \N     | before trigger fired
+  4007 |  7 | XX         | XX     | before trigger fired
+  4008 |  8 | Delimiter  | :      | before trigger fired
+     1 |  1 | stuff      | test_1 | after trigger fired
+     2 |  2 | stuff      | test_2 | after trigger fired
+     3 |  3 | stuff      | test_3 | after trigger fired
+     4 |  4 | stuff      | test_4 | after trigger fired
+     5 |  5 | stuff      | test_5 | after trigger fired
+(25 rows)

 -- COPY w/ oids on a table w/o oids should fail
 CREATE TABLE no_oids (
@@ -80,6 +95,7 @@
 ERROR:  table "no_oids" does not have OIDs
 -- check copy out
 COPY x TO stdout;
+9999    \N    \\N    NN    before trigger fired
 10000    21    31    41    before trigger fired
 10001    22    32    42    before trigger fired
 10002    23    33    43    before trigger fired
@@ -87,12 +103,25 @@
 10004    25    35    45    before trigger fired
 10005    26    36    46    before trigger fired
 6    \N    45    80    before trigger fired
+7    \N    x    \\x    before trigger fired
+8    \N    ,    \\,    before trigger fired
+3000    \N    c    \N    before trigger fired
+4000    \N    C    \N    before trigger fired
+4001    1    empty        before trigger fired
+4002    2    null    \N    before trigger fired
+4003    3    Backslash    \\    before trigger fired
+4004    4    BackslashX    \\X    before trigger fired
+4005    5    N    N    before trigger fired
+4006    6    BackslashN    \\N    before trigger fired
+4007    7    XX    XX    before trigger fired
+4008    8    Delimiter    :    before trigger fired
 1    1    stuff    test_1    after trigger fired
 2    2    stuff    test_2    after trigger fired
 3    3    stuff    test_3    after trigger fired
 4    4    stuff    test_4    after trigger fired
 5    5    stuff    test_5    after trigger fired
 COPY x (c, e) TO stdout;
+\\N    before trigger fired
 31    before trigger fired
 32    before trigger fired
 33    before trigger fired
@@ -100,12 +129,25 @@
 35    before trigger fired
 36    before trigger fired
 45    before trigger fired
+x    before trigger fired
+,    before trigger fired
+c    before trigger fired
+C    before trigger fired
+empty    before trigger fired
+null    before trigger fired
+Backslash    before trigger fired
+BackslashX    before trigger fired
+N    before trigger fired
+BackslashN    before trigger fired
+XX    before trigger fired
+Delimiter    before trigger fired
 stuff    after trigger fired
 stuff    after trigger fired
 stuff    after trigger fired
 stuff    after trigger fired
 stuff    after trigger fired
 COPY x (b, e) TO stdout WITH NULL 'I''m null';
+I'm null    before trigger fired
 21    before trigger fired
 22    before trigger fired
 23    before trigger fired
@@ -113,6 +155,18 @@
 25    before trigger fired
 26    before trigger fired
 I'm null    before trigger fired
+I'm null    before trigger fired
+I'm null    before trigger fired
+I'm null    before trigger fired
+I'm null    before trigger fired
+1    before trigger fired
+2    before trigger fired
+3    before trigger fired
+4    before trigger fired
+5    before trigger fired
+6    before trigger fired
+7    before trigger fired
+8    before trigger fired
 1    after trigger fired
 2    after trigger fired
 3    after trigger fired
diff -ruN ../base/src/test/regress/sql/copy2.sql src/test/regress/sql/copy2.sql
--- ../base/src/test/regress/sql/copy2.sql    2003-05-14 05:26:03.000000000 +0200
+++ src/test/regress/sql/copy2.sql    2003-10-07 20:49:49.000000000 +0200
@@ -2,7 +2,7 @@
     a serial,
     b int,
     c text not null default 'stuff',
-    d text not null,
+    d text,
     e text
 );

@@ -27,6 +27,7 @@
 FOR EACH ROW EXECUTE PROCEDURE fn_x_before();

 COPY x (a, b, c, d, e) from stdin;
+9999    \N    \\N    \NN    \N
 10000    21    31    41    51
 \.

@@ -74,6 +75,24 @@
 -- various COPY options: delimiters, oids, NULL string
 COPY x (b, c, d, e) from stdin with oids delimiter ',' null 'x';
 500000,x,45,80,90
+500001,x,\x,\\x,\\\x
+500002,x,\,,\\\,,\\
+\.
+
+COPY x from stdin WITH DELIMITER AS ';' NULL AS '';
+3000;;c;;
+\.
+
+COPY x from stdin WITH DELIMITER AS ':' NULL AS '\\X';
+4000:\X:C:\X:\X
+4001:1:empty::
+4002:2:null:\X:\X
+4003:3:Backslash:\\:\\
+4004:4:BackslashX:\\X:\\X
+4005:5:N:\N:\N
+4006:6:BackslashN:\\N:\\N
+4007:7:XX:\XX:\XX
+4008:8:Delimiter:\::\:
 \.

 -- check results of copy in

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