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

[Joe Wang]

Lance,

It looks fine to me.

Best,
Joe

On 6/5/2012 7:16 AM, Lance Andersen - Oracle wrote:
> 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
>