Issue1741

classification
Title: com.ziclix.python.sql.DataHandler calls wasNull without previously calling get [patch]
Type: Severity: normal
Components: zxjdbc Versions: Jython 2.5
Milestone:
process
Status: open Resolution: remind
Dependencies: Superseder:
Assigned To: Nosy List: fwierzbicki, swhite, zyasoft
Priority: normal Keywords: patch

Created on 2011-04-23.23:28:05 by swhite, last changed 2014-10-05.16:39:17 by zyasoft.

Files
File name Uploaded Description Edit Remove
zxjdbc-wasnull.patch swhite, 2011-04-23.23:28:04 potential patch (fix + test)
Messages
msg6513 (view) Author: Stephen White (swhite) Date: 2011-04-23.23:28:04
If the column type in the ResultSet is reported as NULL then the DataHandler code doesn't bother trying to call any of the get methods, as the value must be NULL.  Unfortunately it does then call wasNull().  This is invalid (see the Note in the Javadocs for java.sql.ResultSet), you can only use wasNull if you've previously called a get method.

There are several easy fixes to this.  One would be to call a get method, even though we know what the returned value will be.  I've implemented an alternative, that bypasses the problematic wasNull check by returning from the middle of the switch statement.  Another approach would be to put a guard around the 'wasNull' call below the switch statement, avoiding calling it if the value is already null or Py.None.

This issue causes real problems when using the org.sqlite.JDBC driver (both the zentus and xerial implementations seems to cause problems).  The patch includes a test that shows the problematic case, though I can only get it to fail with sqlite and not with other databases.  I had to add sqlite to test.xml to run the test, and sqlite doesn't pass all the other tests.
msg6514 (view) Author: Stephen White (swhite) Date: 2011-04-24.01:32:26
Sadly, although the patch fixes an obvious problem (the Type.NULL code is clearly wrong and should be fixed) it doesn't fix all the problems that occur when using this code with sqlite.

It looks like the column type reported through the metadata from sqlite changes based on the stored value, not based on the type name or type affinity of the column.

This means that that, although the patch fixes the real-world situation where sqlite returns a single row containing a NULL, it doesn't fix the case that there are multiple returned rows the first of which contains NULL.  This causes sqlite to return Types.NULL for the first row, and not unreasonably DataHandler.java assumes this type applies to all the values in the column.

A test along the following lines still fails under sqlite, even with the patch.

def testNullReturnQuery(self):
     """testing that a resultset containing a NULL value doesn't break."""
     c = self.cursor()
     try:
         c.execute("insert into zxtesting (id, name, state) values (100, NULL, 'xx')")
         self.db.commit()
         c.execute("select name from zxtesting where id = 100 or id = 1 order by id desc");
         f = c.fetchall()
         assert len(f) == 2, "expecting two rows"
         data = f[0]
         assert data[0] is None, "expecting None/NULL returned, was %r" % (data,)
         data = f[1]
         assert data[0] is not None, "expecting non-NULL value returned, was %r" % (data,)
     finally:
         c.close()

The only fix I can think of is to re-query the type for each value returned, i.e. rather than pass the type parameter into the getPyObject method, have it obtain it each time via something like:

int type = stmt.getMetaData().getColumnType(col)

This doesn't seem an ideal thing to have to do in generic JDBC driver code but it does make things work better for sqlite case.
msg9067 (view) Author: Jim Baker (zyasoft) Date: 2014-10-05.16:39:17
Revisit with zxJDBC to jyjdbc migration
History
Date User Action Args
2014-10-05 16:39:17zyasoftsetnosy: + zyasoft
messages: + msg9067
2013-02-20 00:10:42fwierzbickisetversions: + Jython 2.5, - 2.5.2rc
2013-02-20 00:00:07fwierzbickisetpriority: normal
resolution: remind
2012-08-10 21:08:34fwierzbickisetnosy: + fwierzbicki
2011-04-24 01:32:26swhitesetmessages: + msg6514
2011-04-23 23:28:05swhitecreate