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.ander...@oracle.com>
Date: May 26, 2012 9:33:40 AM EDT
Cc: core-libs-dev core-libs-dev<core-libs-dev@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.ander...@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.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

Reply via email to