db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bryan Pendleton (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-4062) Method override missing from DataValueFactory interface
Date Tue, 17 Feb 2009 19:26:59 GMT

    [ https://issues.apache.org/jira/browse/DERBY-4062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12674323#action_12674323
] 

Bryan Pendleton commented on DERBY-4062:
----------------------------------------

Thanks for tracking the commit down, Knut. That change occurred
after the DERBY-2487 patch was originally developed, which
helps explain why the original developer of that patch didn't notice this.

I'm a little confused about why the change removed some of 
the DVF single-argument API overloads, but not all of them.

For example, revision 540791 removed getDataValue(short)
from the DataValueFactory interface, but left getDataValue(Short).

But the reverse change occurred with int/Integer: revision 540791
removed getDataValue(Integer) from the DataValueFactory interface,
but left getDataValue(int). So that's confusing.

Also, the change removed the *implementation* of getDataValue(short)
as well as the interface, but for getDataValue(Integer) the method
was removed only from the interface, not from the implementation.

I suppose that one alternative is to leave this alone as is, and to alter
the DERBY-2487 code so that instead of calling getDataValue(Integer),
it calls getDataValue(Integer, NumberDataValue previous), and pass
a NULL value for previous. Thus change:

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

to

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

and similarly for all the other Integer columns in the new system tables.

It's a little bit scary to leave this interface in this strange state where, due
to argument promotion rules, the caller may not be completely clear on
what actual overridden method will be called.



> Method override missing from DataValueFactory interface
> -------------------------------------------------------
>
>                 Key: DERBY-4062
>                 URL: https://issues.apache.org/jira/browse/DERBY-4062
>             Project: Derby
>          Issue Type: Sub-task
>            Reporter: Bryan Pendleton
>            Assignee: Bryan Pendleton
>            Priority: Minor
>         Attachments: addToInterface.diff
>
>
> 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);
>  Because this method is not present in the interface, code such as
>    row.setColumn(SYSXPLAIN_RESULTSET_NO_OPENS, dvf.getDataValue(no_opens));
> which the code anticipates will invoke the above method, instead calls the method
>    public UserDataValue getDataValue(Object value); 
> which has a very different behavior (instead of returning a SQLInteger, it returns a
UserType).
> This accidental invocation of the wrong implementation method was causing data corruption
> errors in regression tests for the DERBY-2487 patch, which uses the above setColumn call.
> Instead of inserting SQLInteger values into the system table, the code was inserting
> java.lang.Integer UserType values; since those values don't match the defined type of
> the column(s) in the system catalog, the table appeared to be corrupt.
> I believe that this problem never affects external Derby applications, but only internal
Derby code,
> as the DataValueFactory interface is an internal interface only. Still, since it appeared
to
> cause data corruption and invalid query results, it is potentially a quite serious problem.
> See this thread in the derby-dev archives for a bit more discussion:
> http://mail-archives.apache.org/mod_mbox/db-derby-dev/200902.mbox/%3C4997818E.3080007@amberpoint.com%3E

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message