Anurag Shekhar (JIRA) wrote: > [ > http://issues.apache.org/jira/browse/DERBY-819?page=comments#action_12366069 > ] > > Anurag Shekhar commented on DERBY-819: > -------------------------------------- > > Description of patch
Thanks for the description, it greatly helps the reviewers. I still haven't managed to download a complete copy of the patch yet and Jira seems down this morning. > Util > Removed all exception related methods except notImplemented. This method is > used from network client. Changed this method to return SQLException. This > methods used only by network client and that is not supposed to used > EmbeddedException. This helped me to understand why your are doing these changes: - throw Util.notImplemented(); + throw InternalDriver.getExceptionFactory().notImplemented(); I couldn't understand before your explanation for a number of reasons: 1) A simpler fix would have been to modify the Util.notImplemented() method and leave all the callers alone. 2) This would have been less work for you 3) This would have been less work for the reviewer 4) It would have resulted in not increasing the footprint. In all the callers of this method you have changed them from one method call to two method calls, hence more footprint. 5) In a way they seem like changes unrelated to the jira description Then I saw your comment about the client. So due to this review and your description we have found a bug in the existing JDBC 4.0 network client. The client code is not meant to be using engine code, all the calls to org.apache.derby.impl.jdbc.Util.notImplemented() in the new JDBC 4.0 classes are wrong. The current state will cause a class not found exception if those methods are called as the Util class correctly is not part of derbyclient.jar. And we expect the client jar to be useable without derby.jar. I'm not sure if there is a utility method for "not implemented" in the client, if there is one that is what the client must use. Thus it seems the changes you are making related to not using the existing Util.notImplemented() method and changing all the callers is actually driven by a bug in the client code. Dan.
