[ https://issues.apache.org/jira/browse/DERBY-6445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17849588#comment-17849588 ]
Richard N. Hillegas commented on DERBY-6445: -------------------------------------------- Attaching derby-6445-01-ab-DERBY-6445.patchPlusPlusTweaks.diff. This adds some additional tweaks to the derby-6445-01-aa-DERBY-6445.patchPlusJavadocCleanup.diff patch. Also attaching tweaks.diff which shows the difference between derby-6445-01-aa-DERBY-6445.patchPlusJavadocCleanup.diff and derby-6445-01-ab-DERBY-6445.patchPlusPlusTweaks.diff. The tweaks are: 1) Diagnostic traces were added for entering and exiting some of the methods. As Philippe points out, there is no ideal approach to this problem. The original tracing was for entering/exiting the JDBC getter and setter methods. This breaks down when dealing with get/setObject() because those methods forward to more specific getters and setters. I added traces to the new forwarding methods. 2) I reduced the visibility of some of the new methods from public to private in order to avoid encroaching on public interfaces which may change in future revs of the JDBC spec. 3) I changed the data types reported in some error messages. This patch touches the same files as the previous patches. I ran full tests using both the classpath and the modulepath. Both runs were clean. I am inclined to commit this patch because, despite my reservations (see below), I think that this is a large piece of solid, incremental work. Having some support for these data types is better than raising exceptions. Thanks, Philippe, for this valuable contribution. My chief reservations are the following: A) I am not an expert on the new date/time classes. I can't evaluate whether the patch causes these types to serialize and deserialize correctly. These classes came out of long, tortured discussions among Java champions and my sense is that the experts failed to reach consensus. I have nothing to add to those discussions. I am worried that an expert may log a bug saying that Derby is not (de)serializing these classes correctly. B) Therefore, I would like to see some comments which explain the approach taken in serializing these objects, that is, for converting between the new classes and the JDBC types which Derby already supports. Right now, there are very few comments in this large patch. The comments should help us field bug reports which may be filed against the correctness of the serialization. C) I would also like to see comments added to the new test methods, explaining what the methods are testing. Thanks, -Rick > JDBC 4.2: Add support for new date and time classes > --------------------------------------------------- > > Key: DERBY-6445 > URL: https://issues.apache.org/jira/browse/DERBY-6445 > Project: Derby > Issue Type: Improvement > Components: JDBC > Affects Versions: 10.10.1.1 > Reporter: Knut Anders Hatlen > Priority: Major > Attachments: DERBY-6445.patch, Derby-6445.html, Derby-6445.html, > derby-6445-01-aa-DERBY-6445.patchPlusJavadocCleanup.diff, > derby-6445-01-ab-DERBY-6445.patchPlusPlusTweaks.diff, tweaks.diff > > > JDBC 4.2 added type mappings for new date and time classes found in Java 8. > Derby should support these new mappings. > This would at least affect Derby's implementation of the various getObject(), > setObject() and setNull() methods in ResultSet, PreparedStatement and > CallableStatement. -- This message was sent by Atlassian Jira (v8.20.10#820010)