db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bryan Pendleton <bpendle...@amberpoint.com>
Subject Re: Looking for a clue or a theory
Date Mon, 16 Feb 2009 17:54:09 GMT
>> I'm studying the DERBY-2487 patch proposal, and have encountered some
>> strange behavior that I don't understand. 

Thanks Knut for the suggestions, although I think the problem was elsewhere.

I think I've found the problem. It was very strange and subtle, and so
I'd like to describe it here and ask for advice.

I believe the problem involves o.a.d.iapi.types.DataValueFactory.
This interface defines dozens and dozens of overloads of the method
getDataValue(), for lots of different combinations of datatypes.

For most of the Java "boxed" types (Short, Long, Float, Double, etc.),
DataValueFactory defines a pair of getDataValue() methods. For example,
here are the method pair that the interface defines for Short:

         /**
          * Get a SQL smallint with the given value.  A null argument means get
          * a SQL null value.  The second form uses the previous value (if non-null)
          * to hold the return value.
          *
          */
         NumberDataValue         getDataValue(Short value);
         NumberDataValue         getDataValue(Short value, NumberDataValue previous)
                                                         throws StandardException;

HOWEVER, for the Integer type, DataValueFactory doesn't define both overloads,
but only defines the 'previous'-style overload:

         /**
          * Get a SQL int with the given value.  A null argument means get
          * a SQL null value.  Uses the previous value (if non-null)
          * to hold the return value.
          *
          */
         NumberDataValue         getDataValue(Integer value, NumberDataValue previous)
                                                         throws StandardException;

The actual implementation, in o.a.d.iapi.types.DataValueFactoryImpl, though,
does implement both the Integer overloads. But this method is NOT present
in the DataValueFactory interface:

         NumberDataValue         getDataValue(Integer value);

This was causing trouble with the following code in the new Derby-2487 patch:

         row.setColumn(SYSXPLAIN_RESULTSET_NO_OPENS, dvf.getDataValue(no_opens));

The no_opens variable is an Integer, and so the code was expecting to call
DataValueFactoryImpl.getDataValue(Integer value), which returns a SQLInteger.

However, since this method was missing from the DataValueFactory interface, the
compiler was instead silently choosing the more generic method from the
DataValueFactory interface:

         public UserDataValue getDataValue(Object value);

and the implementation of this method returns not a SQLInteger, but a UserType:

         public UserDataValue getDataValue(Object value)
         {
                 return new UserType(value);
         }

The bottom line was that instead of storing SQLInteger objects into the system
catalog tables for the XPLAIN data, the code was storing UserType(java.lang.integer)
objects, which was corrupting the data because the table descriptor stated that
the columns were SQLInteger types. (Unfortunately, it was a rather quiet corruption;
I'm surprised that this didn't cause more severe symptoms.)

I believe that the fix may be as simple as adding this apparently-omitted method
to the DataValueFactory interface:

svn diff ./engine/org/apache/derby/iapi/types/DataValueFactory.java
Index: engine/org/apache/derby/iapi/types/DataValueFactory.java
===================================================================
--- engine/org/apache/derby/iapi/types/DataValueFactory.java    (revision 744244)
+++ engine/org/apache/derby/iapi/types/DataValueFactory.java    (working copy)
@@ -46,6 +46,7 @@
           * to hold the return value.
           *
           */
+        NumberDataValue         getDataValue(Integer value) throws StandardException;
          NumberDataValue         getDataValue(Integer value, NumberDataValue previous)
                                                          throws StandardException;

HOWEVER, perhaps this method was intentionally omitted? Is there a reason that
this method was missing?

For now, I'll experiment with adding this method and running all the regression
tests, but if anybody knows anything about why this method was missing from
the interface and whether that was important, please let me know.

thanks,

bryan



Mime
View raw message