Daniel John Debrunner wrote:
In some ways Cloudscape split too much into api and implementation and the types package was attempt to fix that, there used to be an impl types. Looking at it, Derby will only ever need one implementation for INTEGER so that's why the types moved to a single package. In only rare cases, DECIMAL, maybe XML does Derby need separate implementations.
Okay, I can move impl.sql.xml.* back to the iapi.types. But will that resolve the issue you mentioned about spilling over into the SQL layer? Even if I redo the packaging, the XML implementation file will still make the call to the DataDictionary class in org.apache.derby.iapi.sql.dictionary, so isn't that cross-over still going to be there? Does that part of the code need to change?
There is a theory of "You ain't going to need it", which really says only code what you need. If we only need a single XML implementation at the moment then code it that way, just allow the on-disk format to have multiple implementations in the future. The api is XMLDataValue and the implementation XML. Just an idea, again I think in Cloudscape we built too much infrastructure for multiple implementations, where in many cases one will suffice.
I can certainly code it for a single XML implementation, but seeing as, in this particular case, we _expect_ that multiple implementations are going to exist for XML, it seems much cleaner (to me) to code up the structure now. When the SQLInteger type was created, what were the odds that it was going to require multiple implementations? Probably not as high as for XML--not only are we acknowledging that another (better) XML implementation could come up in the future, but we're _hoping_ for one. To put the structure in place now means that future contributors can focus on adding the new implementation and not on re-organizing the old one(s). And the easier it is to add a new, smarter implementation, the more likely we are to have someone volunteer to do it :)
But that's my proverbial two cents. I can certainly re-org the code to just have an XMLDataValue interface and a single XML implementation, if that's the desired solution.
SO, I guess I'll post the revised patch that I have (all tests passed from last night's run) for people to review, and then I'll start working on a third version of the patch that 1) moves impl.sql.xml back to types.iapi, and 2) is simplified to just worry about a single XML (UTF-8 based) implementation.
Thanks much for the feedback! Army
