RFR: 8085984 Add JDBC Sharding API
Lance Andersen
lance.andersen at oracle.com
Tue Nov 24 16:47:46 UTC 2015
See: http://cr.openjdk.java.net/~lancea/8085984/webrev.01/
On Nov 24, 2015, at 10:31 AM, Paul Benedict <pbenedict at apache.org> wrote:
> Some comments:
>
> General
> *) Shouldn't the javadocs be put mentions of null in {@code null} tags?
Probably, JDBC javadocs are inconsistent WRT null (and <code> vs {@code} so it needs a general cleanup. However I made this change for these methods as needed
> *) Many methods are documented to throw SQLFeatureNotSupportedException when sharding isn't supported, but the error message actually says "XYZ not implemented". Well I think that places an immediate burden on the driver implementors to correct the error message. If it's simply not supported by default, the message should simply be "XYZ does not support sharding" (or something similar) rather than talk about lack of implementation. WDYT?
I will think about this but do not plan to update this prior to pushing
>
> XADataSource
> *) createXAConnectionBuilder needs its error message fixed. Add "XA" in the string
addressed
> *) I noticed the closing method braces don't align with "default" keyword
addressed and in DataSource
>
> ConnectionBuilder/XAConnectionBuilder
> *) shardingKey() and superShardingKey() could benefit from a {@see} to ShardingKeyBuilder since ShardingKeyBuilder actually explains what a [super]sharding key is. I actually prefer the explanation of what these keys are to be copied into these methods, but a {@see} would be the minimal.
Added @see. Copying the wording everywhere makes it easy to miss when updating.
>
> ShardingKey/ShardingKeyBuilder/ConnectionBuilder/XAConnectionBuilder
> *) All the examples use createShardingKeyBuilder() but that method is not guaranteed to be implemented. Don't you think that might make the examples misleading?
They are examples and not meant to be exhaustive. Perhaps later I can look to add additional examples.
Best
Lance
>
> Cheers,
> Paul
>
> On Mon, Nov 23, 2015 at 7:21 PM, Lance Andersen <lance.andersen at oracle.com> wrote:
> Hi Joe,
>
> Thank you and Ulf for catching the error message typo as we changed the name of the method. I will fix that before I push
>
> Best
> Lance
> On Nov 23, 2015, at 7:09 PM, huizhe wang <huizhe.wang at oracle.com> wrote:
>
> > Hi Lance,
> >
> > Looks good, except as Ulf pointed out already, in both in Connection.java and XAConnection.java, SQLFeatureNotSupportedException should have been thrown with a message "setShardingKeyIfValid not implemented" rather than "isValid not implemented" (4 occurrences in total).
> >
> > Best,
> > Joe
> >
> > On 11/23/2015 3:01 PM, Lance Andersen wrote:
> >> Hi all,
> >>
> >> Need a reviewer for 8085984, which adds support for Sharding. The CCC has been approved. The webrev can be found at http://cr.openjdk.java.net/~lancea/8085984/webrev.00/
> >>
> >> Best,
> >> Lance
> >>
> >>
> >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> >> Oracle Java Engineering
> >> 1 Network Drive
> >> Burlington, MA 01803
> >> Lance.Andersen at oracle.com
> >>
> >>
> >>
> >
>
>
>
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com
>
>
>
>
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com
More information about the core-libs-dev
mailing list