Fwd: Review request for CR 171917 CachedRowSetImpl.populate does not handle map properly

[Lance Andersen - Oracle]

Looking for a reviewer here as well still

Best
Lance

Begin forwarded message:

> From: Lance Andersen - Oracle <Lance.Andersen at oracle.com>
> Date: May 26, 2012 9:33:40 AM EDT
> Cc: core-libs-dev core-libs-dev <core-libs-dev at openjdk.java.net>
> Subject: Re: Review request for CR 171917 CachedRowSetImpl.populate does not handle map properly
> 
> Here is the revised change with David's suggestion. All tests continue to pass
> 
> 
> new-host-4:rowset lanceandersen$ !hg
> hg diff CachedRowSetImpl.java 
> diff -r 4580652d9828 src/share/classes/com/sun/rowset/CachedRowSetImpl.java
> --- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.java	Fri May 04 16:00:47 2012 -0400
> +++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.java	Sat May 26 08:43:15 2012 -0400
> @@ -659,7 +659,7 @@
>                  * us work with drivers that do not support
>                  * getObject with a map in fairly sensible way
>                  */
> -                if (map == null) {
> +                if (map == null || map.isEmpty()) {
>                     obj = data.getObject(i);
>                 } else {
>                     obj = data.getObject(i, map);
> 
> 
> Best
> Lance
> On May 25, 2012, at 7:35 PM, David Schlosnagle wrote:
> 
>> On Fri, May 25, 2012 at 3:50 PM, Lance Andersen - Oracle
>> <Lance.Andersen at oracle.com> wrote:
>>> The populate() method needs to check for a size of 0 for the map in case a webrowset xml file has an empty map tag,  which would result in calling setobject specifying a map and not all databases/drivers support this.
>>> 
>>> simple 1 line change:
>>> 
>>> hg diff CachedRowSetImpl.java
>>> diff -r 4580652d9828 src/share/classes/com/sun/rowset/CachedRowSetImpl.java
>>> --- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.java    Fri May 04 16:00:47 2012 -0400
>>> +++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.java    Fri May 25 15:45:29 2012 -0400
>>> @@ -659,7 +659,7 @@
>>>                 * us work with drivers that do not support
>>>                 * getObject with a map in fairly sensible way
>>>                 */
>>> -                if (map == null) {
>>> +                if (map == null || map.size() == 0) {
>> 
>> Lance,
>> 
>> Is there any reason not to use Map.isEmpty() which would be useful if
>> the Map has an expensive size() method?
>> 
>> -                if (map == null) {
>> +                if (map == null || map.isEmpty()) {
>> 
>> Thanks,
>> Dave
> 
> 
> 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