Fix COPY FROM for null marker strings that correspond to invalid encoding.

The COPY documentation says "COPY FROM matches the input against the null
string before removing backslashes".  It is therefore reasonable to presume
that null markers like E'\\0' will work ... and they did, until someone put
the tests in the wrong order during microoptimization-driven rewrites.
Since then, we've been failing if the null marker is something that would
de-escape to an invalidly-encoded string.  Since null markers generally
need to be something that can't appear in the data, this represents a
nontrivial loss of functionality; surprising nobody noticed it earlier.

Per report from Jeff Davis.  Backpatch to 8.4 where this got broken.

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index c464ed7f6ce16dfed49104c236cfcfb3f1f59f3e..eca25b1f64113cee1c920a4b86a1a9a454f5f292 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2730,7 +2730,17 @@ CopyReadAttributesText(CopyState cstate, int maxfields, char **fieldvals)
                start_ptr = cur_ptr;
                fieldvals[fieldno] = output_ptr;
 
-               /* Scan data for field */
+               /*
+                * Scan data for field.
+                *
+                * Note that in this loop, we are scanning to locate the end of field
+                * and also speculatively performing de-escaping.  Once we find the
+                * end-of-field, we can match the raw field contents against the null
+                * marker string.  Only after that comparison fails do we know that
+                * de-escaping is actually the right thing to do; therefore we *must
+                * not* throw any syntax errors before we've done the null-marker
+                * check.
+                */
                for (;;)
                {
                        char            c;
@@ -2843,26 +2853,29 @@ CopyReadAttributesText(CopyState cstate, int maxfields, char **fieldvals)
                        *output_ptr++ = c;
                }
 
-               /* Terminate attribute value in output area */
-               *output_ptr++ = '\0';
-
-               /*
-                * If we de-escaped a non-7-bit-ASCII char, make sure we still have
-                * valid data for the db encoding. Avoid calling strlen here for the
-                * sake of efficiency.
-                */
-               if (saw_non_ascii)
-               {
-                       char       *fld = fieldvals[fieldno];
-
-                       pg_verifymbstr(fld, output_ptr - (fld + 1), false);
-               }
-
                /* Check whether raw input matched null marker */
                input_len = end_ptr - start_ptr;
                if (input_len == cstate->null_print_len &&
                        strncmp(start_ptr, cstate->null_print, input_len) == 0)
                        fieldvals[fieldno] = NULL;
+               else
+               {
+                       /*
+                        * At this point we know the field is supposed to contain data.
+                        *
+                        * If we de-escaped any non-7-bit-ASCII chars, make sure the
+                        * resulting string is valid data for the db encoding.
+                        */
+                       if (saw_non_ascii)
+                       {
+                               char       *fld = fieldvals[fieldno];
+
+                               pg_verifymbstr(fld, output_ptr - fld, false);
+                       }
+               }
+
+               /* Terminate attribute value in output area */
+               *output_ptr++ = '\0';
 
                fieldno++;
                /* Done if we hit EOL instead of a delim */

diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 7f374ac1a6f79e41dc530501ed85254d0db9864d..c9d575945685234d1638ac40c8b95b16d436e9db 100644 (file)
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -202,6 +202,22 @@ a\.
 \.b
 c\.d
 "\."
+-- test handling of nonstandard null marker that violates escaping rules
+CREATE TEMP TABLE testnull(a int, b text);
+INSERT INTO testnull VALUES (1, E'\\0'), (NULL, NULL);
+COPY testnull TO stdout WITH NULL AS E'\\0';
+1      \\0
+\0     \0
+COPY testnull FROM stdin WITH NULL AS E'\\0';
+SELECT * FROM testnull;
+ a  | b  
+----+----
+  1 | \0
+    | 
+ 42 | \0
+    | 
+(4 rows)
+
 DROP TABLE x, y;
 DROP FUNCTION fn_x_before();
 DROP FUNCTION fn_x_after();

diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 7c23ba253c4e28a47af195963ffd3d8094b829da..09a4fa633a9eb1a15f76a7a434781e95274cc51b 100644 (file)
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -151,6 +151,21 @@ c\.d
 
 COPY testeoc TO stdout CSV;
 
+-- test handling of nonstandard null marker that violates escaping rules
+
+CREATE TEMP TABLE testnull(a int, b text);
+INSERT INTO testnull VALUES (1, E'\\0'), (NULL, NULL);
+
+COPY testnull TO stdout WITH NULL AS E'\\0';
+
+COPY testnull FROM stdin WITH NULL AS E'\\0';
+42     \\0
+\0     \0
+\.
+
+SELECT * FROM testnull;
+
+
 DROP TABLE x, y;
 DROP FUNCTION fn_x_before();
 DROP FUNCTION fn_x_after();