See: http://cr.openjdk.java.net/~lancea/8085984/webrev.01/
On Nov 24, 2015, at 10:31 AM, Paul Benedict <pbened...@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.ander...@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.w...@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.ander...@oracle.com
> >>
> >>
> >>
> >
>
>
>
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> lance.ander...@oracle.com
>
>
>
>
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
