7116445: Miscellaneous warnings in the JDBC/RowSet classes

Fri Dec 2 20:45:15 UTC 2011

Hi Lance,

I'm OK with postponing the @Deprecated work, and doing a separate pass for 
@Deprecated, including the com.sun.* stuff at that time.

There's enough stuff in this changeset already. I think we're better off 
getting it in now than putting more stuff in and getting it reviewed again. I 
think you've done this to yourself on this review twice already. :-)

Comments on webrev.03 follow.

CachedRowSetImpl.java --

451     @SuppressWarnings("rawtypes")
452     public CachedRowSetImpl(Hashtable env) throws SQLException {

The warning suppression covers the entire constructor. You could do this...

     public CachedRowSetImpl(@SuppressWarnings("rawtypes") Hashtable env)

Ugh. As you said earlier, probably not worth spending more cycles on this.

5710    Class<?> c = (Class)map.get(s.getSQLTypeName());

The cast to (Class) is probably unnecessary since the values in the map are 
already Class<?>.

JoinRowSetImpl.java --

932                (vecRowSetsInJOIN.get(i)).getMatchColumnNames()[0]);
4179         Integer i = (vecJoinType.get(vecJoinType.size()-1));

Can probably remove extra set of parens that were necessary when the cast was 
there.

CachedRowSetWriter.java --

576                     c = (Class)map.get(s.getSQLTypeName());

Redundant cast to (Class).

BaseRowSet.java --

622             for (Iterator<?> i = listeners.iterator(); i.hasNext(); ) {
623                 ((RowSetListener)i.next()).cursorMoved(event);
624             }

Hm, listeners is Vector<RowSetListener> so some casts could be removed,

622             for (Iterator<RowSetListener> i = listeners.iterator(); 
i.hasNext(); ) {
623                 (i.next()).cursorMoved(event);
624             }

or even changed to

622             for (RowSetListener rsl : listeners) {
623                 rsl.cursorMoved(event);
624             }

There are several other cases in this file where you used Iterator<?>. You 
might check them to see if similar simplifications could be done there as well.

(There is also Iterator<?> used in CachedRowSetImpl.java but this is used to 
iterate over rvh, a Vector<Object>, but then the elements are cast to Row. This 
begs the question of why rvh isn't Vector<Row> but this is beyond the scope of 
warnings cleanup.)

SerialArray.java --

290         default:
291             ;

I'm surprised you didn't remove these lines as you had done above.

*******

With the exception of changing Iterator<?> to use a for-each loop in 
BaseRowSet.java, all of these comments are just little tweaks, so I don't think 
we need another webrev for them. It's up to you whether to proceed with the 
BaseRowSet changes I recommend. Regardless, I don't want to see another webrev 
of this code. Check it in!! :-)

s'marks

On 12/2/11 8:06 AM, Lance Andersen - Oracle wrote:
>
> On Dec 2, 2011, at 10:54 AM, David Schlosnagle wrote:
>
>>   On Fri, Dec 2, 2011 at 8:19 AM, Lance Andersen - Oracle<Lance.Andersen at oracle.com>  wrote:
>> Adding @Deprecated changes the signatures so I need to coordinate any changes as it will result in TCK signature failures.  This is something I will most likely do as part of the JDBC 4.2 work after giving the JDBC EG a chance for input.
>>
>> Lance,
>>
>> I figured it was worth a shot to try and piggy back on your changes to cleanup the rest of java.sql.* :)
>
> Appreciate the suggestion.
>
>>
>> I assume it is the @Deprecated annotation's retention runtime value that introduces TCK signature compatibility issues, is that correct?
>
> Yes, here is the reason why:
>
>
> @Documented
> @Retention(value=RUNTIME)
> @Target(value={CONSTRUCTOR,FIELD,LOCAL_VARIABLE,METHOD,PACKAGE,PARAMETER,TYPE})
> public @interface Deprecated
>
> why  Here is @Documented
>
>
> @Documented
> @Retention(value=RUNTIME)
> @Target(value=ANNOTATION_TYPE)
> public @interface Documented
> Indicates that annotations with a type are to be documented by javadoc and similar tools by default. This type should be used to annotate the declarations of types whose annotations affect the use of annotated elements by their clients. If a type declaration is annotated with Documented, its annotations become part of the public API of the annotated elements.
> Since:
> 1.5
>
> As you can see this results in an API change which would result in the signatures changing.  This type of change would be flagged by the signature tests so we would not want to do this in say JDK 7.  It would be OK for JDK 8 but I would not want to do it as part of the cleanup changes.
>
> It is interesting that the behavior is different between @Deprecated vs @deprecated(javadoc mark up) and I have had previous discussions on this as one vendor made this innocent change in a standalone technology and then could not pass the signature tests for a given TCK.
>
> Again, my desire is to do this soon,  and might for the com.* classes assuming Alan/Stuart see no issue as part of this push
>
> Many thanks for your time
>
> Best
> Lance
>> If so, that is an interesting catch-22 when the idea behind @Deprecated is to maintain a backward compatible API for some period to be determined to the API provider. It seems like this might be worthwhile mentioning in the deprecation guide [1], and maybe even additions to Joe Darcy's excellent treatises on compatibility [2] [3]. I completely understand wanting to wait for the appropriate approvals to move forward with the remaining cleanup.
>>
>>
>> On Fri, Dec 2, 2011 at 9:14 AM, Lance Andersen - Oracle<Lance.Andersen at oracle.com>  wrote:
>> Here is the diff for DriverManager, I won't be pushing another webrev unless the word is to go ahead and add @Deprecated to the com/* classes of the RowSet RI or there is another change requested that is more detailed:
>>
>> That looks good to me, unfortunately I don't think I can be used as a reviewer.
>>
>> Thanks,
>> Dave
>>
>> [1]: http://docs.oracle.com/javase/6/docs/technotes/guides/javadoc/deprecation/deprecation.html
>> [2]: http://blogs.oracle.com/darcy/entry/kinds_of_compatibility
>> [3]: http://blogs.oracle.com/darcy/entry/release_types_compatibility_regions
>>
>
>
> 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
>