Re: Trouble with COPY IN

Matthew Wakeling <matthew@flymine.org>

From: Matthew Wakeling <matthew@flymine.org>
To: Kevin Grittner <Kevin.Grittner@wicourts.gov>
Cc: Maciek Sakrejda <msakrejda@truviso.com>, Samuel Gendler <sgendler@ideasculptor.com>, pgsql-jdbc@postgresql.org
Date: 2010-07-20T14:55:18Z
Lists: pgsql-hackers
On Tue, 20 Jul 2010, Matthew Wakeling wrote:
> So, should the unlocking happen elsewhere, or should endCopy not require a 
> lock?

Or a third option. We should delay the processing of the "Z" (Ready for 
query) message until after the endCopy has been called.

I have still not been able to create a test that provokes this bug. I do 
not understand under what conditions the server will do this, but 
sometimes it will respond with a "C" and a "Z" before the endCopy message 
has actually been sent to it. Maybe it is keying off the "-1" (no further 
rows) part of the binary copy. In any case, I am including a patch here 
which seems to fix the driver for my application. When the driver receives 
a "C" message (command finished), and block is set to false, then we can 
assume that a little later on we will call processCopyResults again very 
soon with block set to true. Instead of setting block to true, finish 
processing of messages in order to delay seeing the "Z" message until 
after the endCopy method has been called.

The patch also contains a simple test for binary copy in, although it does 
not exercise this bug.

Index: org/postgresql/core/v3/QueryExecutorImpl.java
===================================================================
RCS file: 
/cvsroot/jdbc/pgjdbc/org/postgresql/core/v3/QueryExecutorImpl.java,v
retrieving revision 1.48
diff -u -r1.48 QueryExecutorImpl.java
--- org/postgresql/core/v3/QueryExecutorImpl.java	21 Mar 2010 07:13:37 -0000	1.48
+++ org/postgresql/core/v3/QueryExecutorImpl.java	20 Jul 2010 14:48:03 -0000
@@ -931,7 +931,9 @@
                      error = se;
                  }

-                block = true;
+                if (!block) {
+                    endReceiving = true;
+                }
                  break;

              case 'E': // ErrorMessage (expected response to CopyFail)
Index: org/postgresql/test/jdbc2/CopyTest.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/test/jdbc2/CopyTest.java,v
retrieving revision 1.4
diff -u -r1.4 CopyTest.java
--- org/postgresql/test/jdbc2/CopyTest.java	8 Apr 2010 16:18:05 -0000	1.4
+++ org/postgresql/test/jdbc2/CopyTest.java	20 Jul 2010 14:48:03 -0000
@@ -15,6 +15,7 @@
  import java.sql.Statement;
  import java.io.ByteArrayInputStream;
  import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
  import java.io.InputStream;
  import java.io.OutputStream;
  import java.io.PrintStream;
@@ -272,4 +273,35 @@
          assertEquals(0, getCount());
      }

+    public void testBinaryCopyIn() throws Exception {
+        con.setAutoCommit(true);
+        String sql = "COPY copytest FROM STDIN BINARY";
+        ByteArrayOutputStream data = new ByteArrayOutputStream();
+        DataOutputStream dos = new DataOutputStream(data);
+        dos.writeBytes("PGCOPY\n");
+        dos.writeByte(255);
+        dos.writeBytes("\r\n");
+        dos.writeByte(0); // Signature done
+        dos.writeInt(0); // Flags - we aren't supplying OIDS
+        dos.writeInt(0); // Length of header extension
+        for (int row = 0; row < 99700; row++) {
+            dos.writeShort(3); // Three rows
+            dos.writeInt(5); // String length
+            dos.writeByte('H');
+            dos.writeByte('e');
+            dos.writeByte('l');
+            dos.writeByte('l');
+            dos.writeByte('o');
+            dos.writeInt(4); // int length
+            dos.writeInt(row); // Second argument
+            dos.writeInt(-1); // Null third argument
+        }
+        dos.writeShort(-1); // End of rows
+        dos.flush();
+        dos.close();
+        byte[] byteArray = data.toByteArray();
+        //System.err.println("Data length: " + byteArray.length);
+        long count = copyAPI.copyIn(sql, new ByteArrayInputStream(byteArray));
+        assertEquals(99700, count);
+    }
  }



Hope this helps,

Matthew

-- 
 There once was a limerick .sig
 that really was not very big
 It was going quite fine
 Till it reached the fourth line