Обсуждение: Fix resultset results after updateBinaryStream
Hi,
The current UpdateableResultSet tests did not properly test more complex binary data updating.
The following patch makes the test a bit harder and fixes the driver to pass the test.
Index: org/postgresql/test/jdbc2/UpdateableResultTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/UpdateableResultTest.java,v
retrieving revision 1.24
diff -u -r1.24 UpdateableResultTest.java
--- org/postgresql/test/jdbc2/UpdateableResultTest.java 16 Apr 2007 16:36:41 -0000 1.24
+++ org/postgresql/test/jdbc2/UpdateableResultTest.java 20 Jul 2007 22:28:21 -0000
@@ -10,6 +10,8 @@
package org.postgresql.test.jdbc2;
import java.sql.*;
+import java.util.Arrays;
+
import junit.framework.TestCase;
import java.io.InputStream;
@@ -238,25 +240,26 @@
String string = "Hello";
InputStream asi = new ByteArrayInputStream(string.getBytes("US-ASCII"));
Reader chr = new StringReader(string);
- InputStream bin = new ByteArrayInputStream(string.getBytes("US-ASCII"));
+ byte[] bytes = new byte[]{0,'\\',(byte) 128,(byte) 255};
+ InputStream bin = new ByteArrayInputStream(bytes);
rs.updateInt("id", 2);
rs.updateAsciiStream("asi", asi, 5);
rs.updateCharacterStream("chr", chr, 5);
- rs.updateBinaryStream("bin", bin, 5);
+ rs.updateBinaryStream("bin", bin, bytes.length);
rs.updateRow();
assertEquals(2, rs.getInt(1));
assertEquals(string, rs.getString(2));
assertEquals(string, rs.getString(3));
- assertEquals(string, rs.getString(4));
+ assertEquals("Got wrong bytes immediately after update", Arrays.toString(bytes),
Arrays.toString(rs.getBytes(4)));
rs.refreshRow();
assertEquals(2, rs.getInt(1));
assertEquals(string, rs.getString(2));
assertEquals(string, rs.getString(3));
- assertEquals(string, rs.getString(4));
+ assertEquals("Got wrong bytes from database", Arrays.toString(bytes), Arrays.toString(rs.getBytes(4)));
rs.close();
stmt.close();
Index: org/postgresql/jdbc2/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.94
diff -u -r1.94 AbstractJdbc2ResultSet.java
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 16 Apr 2007 16:36:41 -0000 1.94
+++ org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 20 Jul 2007 22:28:21 -0000
@@ -1205,6 +1205,7 @@
if ( rs.next() )
{
+ System.arraycopy(rs.fields, 0, fields, 0, numColumns);
rowBuffer = rs.rowBuffer;
}
@@ -1657,6 +1658,7 @@
{
String columnName = (String) columns.next();
int columnIndex = findColumn( columnName ) - 1;
+ fields[columnIndex].setFormat(Field.TEXT_FORMAT);
Object valueObject = updateValues.get(columnName);
if (valueObject instanceof NullObject)
@@ -1709,6 +1711,13 @@
// Should never happen?
break;
+ case Types.BINARY:
+ case Types.LONGVARBINARY:
+ case Types.VARBINARY:
+ fields[columnIndex].setFormat(Field.BINARY_FORMAT);
+ rowBuffer[columnIndex] = (byte[]) valueObject;
+ break;
+
default:
rowBuffer[columnIndex] = (byte[]) valueObject;
}
On Sat, 21 Jul 2007, Mikko Tiihonen wrote: > The current UpdateableResultSet tests did not properly test more complex > binary data updating. The following patch makes the test a bit harder > and fixes the driver to pass the test. I think this works for the single row update and then retrieve test, but if your ResultSet has two rows and you do update, next, and then getBytes, won't it have the wrong fields[i].getFormat() value? The problem is that we need to keep the format on a per row basis. Consider a scrollable updatable resultset, any row can have either text or binary format. The safest thing to do at the moment would be to encode the byte[] into the text format that the server returns, but this is clearly not an ideal solution. Kris Jurka PS (Great to see you back in action!)
On Sat, 2007-07-21 at 11:40 -0400, Kris Jurka wrote:
>
> On Sat, 21 Jul 2007, Mikko Tiihonen wrote:
>
> > The current UpdateableResultSet tests did not properly test more complex
> > binary data updating. The following patch makes the test a bit harder
> > and fixes the driver to pass the test.
>
> I think this works for the single row update and then retrieve test, but
> if your ResultSet has two rows and you do update, next, and then getBytes,
> won't it have the wrong fields[i].getFormat() value? The problem is that
> we need to keep the format on a per row basis. Consider a scrollable
> updatable resultset, any row can have either text or binary format. The
> safest thing to do at the moment would be to encode the byte[] into the
> text format that the server returns, but this is clearly not an ideal
> solution.
You are correct. Here's an updated patch that does just that. The newer unit
test tests for the new case too. I first tried cloning the "fields" field and
restoring it whenever the row moved. But that did not work when moving to a
row that was inserted or updated. So we really do need either types per row
or convert the fields into correct text/binary format during update.
The actual fix is really hack. But at least it is passes the tests.
Index: org/postgresql/jdbc2/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.94
diff -u -r1.94 AbstractJdbc2ResultSet.java
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 16 Apr 2007 16:36:41 -0000 1.94
+++ org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 21 Jul 2007 20:11:56 -0000
@@ -1709,6 +1709,20 @@
// Should never happen?
break;
+ case Types.BINARY:
+ case Types.LONGVARBINARY:
+ case Types.VARBINARY:
+ if (fields[columnIndex].getFormat() == Field.BINARY_FORMAT) {
+ rowBuffer[columnIndex] = (byte[]) valueObject;
+ } else {
+ try {
+ rowBuffer[columnIndex] = PGbytea.toPGString((byte[]) valueObject).getBytes("ISO-8859-1");
+ } catch (UnsupportedEncodingException e) {
+ e.printStackTrace();
+ }
+ }
+ break;
+
default:
rowBuffer[columnIndex] = (byte[]) valueObject;
}
Index: org/postgresql/test/jdbc2/UpdateableResultTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/jdbc2/UpdateableResultTest.java,v
retrieving revision 1.24
diff -u -r1.24 UpdateableResultTest.java
--- org/postgresql/test/jdbc2/UpdateableResultTest.java 16 Apr 2007 16:36:41 -0000 1.24
+++ org/postgresql/test/jdbc2/UpdateableResultTest.java 21 Jul 2007 20:11:56 -0000
@@ -10,6 +10,8 @@
package org.postgresql.test.jdbc2;
import java.sql.*;
+import java.util.Arrays;
+
import junit.framework.TestCase;
import java.io.InputStream;
@@ -217,6 +219,9 @@
public void testUpdateStreams() throws SQLException, UnsupportedEncodingException
{
+ String string = "Hello";
+ byte[] bytes = new byte[]{0,'\\',(byte) 128,(byte) 255};
+
Statement stmt = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
ResultSet rs = stmt.executeQuery("SELECT id, asi, chr, bin FROM stream");
@@ -227,6 +232,13 @@
rs.updateBinaryStream("bin", null, 0);
rs.insertRow();
+ rs.moveToInsertRow();
+ rs.updateInt(1, 3);
+ rs.updateAsciiStream("asi", new ByteArrayInputStream(string.getBytes("US-ASCII")), 5);
+ rs.updateCharacterStream("chr", new StringReader(string), 5);
+ rs.updateBinaryStream("bin", new ByteArrayInputStream(bytes), bytes.length);
+ rs.insertRow();
+
rs.beforeFirst();
rs.next();
@@ -235,29 +247,31 @@
assertNull(rs.getString(3));
assertNull(rs.getBytes(4));
- String string = "Hello";
- InputStream asi = new ByteArrayInputStream(string.getBytes("US-ASCII"));
- Reader chr = new StringReader(string);
- InputStream bin = new ByteArrayInputStream(string.getBytes("US-ASCII"));
-
rs.updateInt("id", 2);
- rs.updateAsciiStream("asi", asi, 5);
- rs.updateCharacterStream("chr", chr, 5);
- rs.updateBinaryStream("bin", bin, 5);
+ rs.updateAsciiStream("asi", new ByteArrayInputStream(string.getBytes("US-ASCII")), 5);
+ rs.updateCharacterStream("chr", new StringReader(string), 5);
+ rs.updateBinaryStream("bin", new ByteArrayInputStream(bytes), bytes.length);
rs.updateRow();
assertEquals(2, rs.getInt(1));
assertEquals(string, rs.getString(2));
assertEquals(string, rs.getString(3));
- assertEquals(string, rs.getString(4));
+ assertEquals(Arrays.toString(bytes), Arrays.toString(rs.getBytes(4)));
rs.refreshRow();
assertEquals(2, rs.getInt(1));
assertEquals(string, rs.getString(2));
assertEquals(string, rs.getString(3));
- assertEquals(string, rs.getString(4));
+ assertEquals(Arrays.toString(bytes), Arrays.toString(rs.getBytes(4)));
+
+ rs.next();
+ assertEquals(3, rs.getInt(1));
+ assertEquals(string, rs.getString(2));
+ assertEquals(string, rs.getString(3));
+ assertEquals(Arrays.toString(bytes), Arrays.toString(rs.getBytes(4)));
+
rs.close();
stmt.close();
}
On Sat, 21 Jul 2007, Mikko Tiihonen wrote: > On Sat, 2007-07-21 at 11:40 -0400, Kris Jurka wrote: >> >> On Sat, 21 Jul 2007, Mikko Tiihonen wrote: >> >>> The current UpdateableResultSet tests did not properly test more complex >>> binary data updating. The following patch makes the test a bit harder >>> and fixes the driver to pass the test. >> >> I think this works for the single row update and then retrieve test, but >> if your ResultSet has two rows and you do update, next, and then getBytes, >> won't it have the wrong fields[i].getFormat() value? The problem is that >> we need to keep the format on a per row basis. Consider a scrollable >> updatable resultset, any row can have either text or binary format. The >> safest thing to do at the moment would be to encode the byte[] into the >> text format that the server returns, but this is clearly not an ideal >> solution. > > You are correct. Here's an updated patch that does just that. The newer unit > test tests for the new case too. I first tried cloning the "fields" field and > restoring it whenever the row moved. But that did not work when moving to a > row that was inserted or updated. So we really do need either types per row > or convert the fields into correct text/binary format during update. > > The actual fix is really hack. But at least it is passes the tests. > Fixed in CVS for 8.0, 8.1, 8.2, and HEAD. Kris Jurka