geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jasonhu...@apache.org
Subject incubator-geode git commit: GEODE-105: Null value in Map causes NPE with Map Indexes
Date Tue, 11 Aug 2015 21:07:54 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/develop 18bbd9b80 -> fcd03406c


GEODE-105: Null value in Map causes NPE with Map Indexes

Convert null to NULL tokens when saving to the map indexes
We now ignore non map objects instead of throwing assertion errors.
We do not try to index them for map indexes.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/fcd03406
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/fcd03406
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/fcd03406

Branch: refs/heads/develop
Commit: fcd03406c13d8f22b0222b337c6309ab94fce69c
Parents: 18bbd9b
Author: Jason Huynh <jhuynh@pivotal.io>
Authored: Tue Aug 11 14:07:01 2015 -0700
Committer: Jason Huynh <jhuynh@pivotal.io>
Committed: Tue Aug 11 14:07:01 2015 -0700

----------------------------------------------------------------------
 .../query/internal/index/AbstractMapIndex.java  |  6 +--
 .../internal/index/CompactMapRangeIndex.java    |  7 ++-
 .../MapRangeIndexMaintenanceJUnitTest.java      | 50 ++++++++++++++++++++
 3 files changed, 57 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fcd03406/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/AbstractMapIndex.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/AbstractMapIndex.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/AbstractMapIndex.java
index 599e648..198b6ae 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/AbstractMapIndex.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/AbstractMapIndex.java
@@ -319,10 +319,9 @@ public abstract class AbstractMapIndex extends AbstractIndex
   void addMapping(Object key, Object value, RegionEntry entry)
       throws IMQException
   {
-    if(key == QueryService.UNDEFINED) {
+    if(key == QueryService.UNDEFINED || !(key instanceof Map)) {
       return;
     }
-    assert key instanceof Map;
     if (this.isAllKeys) {
       Iterator<Map.Entry<?, ?>> entries = ((Map)key).entrySet().iterator();
       while (entries.hasNext()) {
@@ -346,10 +345,9 @@ public abstract class AbstractMapIndex extends AbstractIndex
   void saveMapping(Object key, Object value, RegionEntry entry)
       throws IMQException
   {
-    if(key == QueryService.UNDEFINED) {
+    if(key == QueryService.UNDEFINED || !(key instanceof Map)) {
       return;
     }
-    assert key instanceof Map;
     if (this.isAllKeys) {
       Iterator<Map.Entry<?, ?>> entries = ((Map)key).entrySet().iterator();
       while (entries.hasNext()) {

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fcd03406/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
index 299ca4f..f8c5745 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactMapRangeIndex.java
@@ -90,10 +90,9 @@ public class CompactMapRangeIndex extends AbstractMapIndex
   void saveMapping(Object key, Object value, RegionEntry entry)
       throws IMQException
   {
-    if(key == QueryService.UNDEFINED) {
+    if(key == QueryService.UNDEFINED || !(key instanceof Map)) {
       return;
     }
-    assert key instanceof Map;
     if (this.isAllKeys) {
       Iterator<Map.Entry<?, ?>> entries = ((Map)key).entrySet().iterator();
       while (entries.hasNext()) {
@@ -107,6 +106,7 @@ public class CompactMapRangeIndex extends AbstractMapIndex
       for (Object mapKey : mapKeys) {
         Object indexKey = ((Map)key).get(mapKey);
         if (indexKey != null) {
+          //Do not convert to IndexManager.NULL.  We are only interested in specific keys
           this.saveIndexAddition(mapKey, indexKey, value, entry);
         }
       }
@@ -156,6 +156,9 @@ public class CompactMapRangeIndex extends AbstractMapIndex
   protected void saveIndexAddition(Object mapKey, Object indexKey, Object value,
       RegionEntry entry) throws IMQException
   {
+    if (indexKey == null) {
+      indexKey = IndexManager.NULL;
+    }
     boolean isPr = this.region instanceof BucketRegion;
     // Get RangeIndex for it or create it if absent
     CompactRangeIndex rg = (CompactRangeIndex) this.mapKeyToValueIndex.get(mapKey);

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/fcd03406/gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
b/gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
index c339ec9..0bb92d3 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
@@ -23,11 +23,13 @@ import org.junit.experimental.categories.Category;
 
 import com.gemstone.gemfire.cache.AttributesFactory;
 import com.gemstone.gemfire.cache.Region;
+import com.gemstone.gemfire.cache.RegionShortcut;
 import com.gemstone.gemfire.cache.Scope;
 import com.gemstone.gemfire.cache.query.CacheUtils;
 import com.gemstone.gemfire.cache.query.Index;
 import com.gemstone.gemfire.cache.query.IndexMaintenanceException;
 import com.gemstone.gemfire.cache.query.QueryService;
+import com.gemstone.gemfire.cache.query.SelectResults;
 import com.gemstone.gemfire.cache.query.data.Portfolio;
 import com.gemstone.gemfire.test.junit.categories.IntegrationTest;
 
@@ -301,7 +303,55 @@ public class MapRangeIndexMaintenanceJUnitTest{
     // recreate index to verify they get updated correctly
     keyIndex1 = (IndexProtocol) qs.createIndex(INDEX_NAME, "positions['SUN', 'IBM']", "/portfolio
");
     assertTrue("Index should be a CompactMapRangeIndex ", keyIndex1 instanceof CompactMapRangeIndex);
+  }
+
+  @Test
+  public void testNullMapValuesInIndexOnLocalRegionForCompactMap() throws Exception{
+    region = CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+    qs = CacheUtils.getQueryService();
+    keyIndex1 = (IndexProtocol) qs.createIndex(INDEX_NAME, "positions[*]", "/portfolio ");
+
+    Portfolio p = new Portfolio(1, 1);
+    p.positions = new HashMap();
+    region.put(1, p);
+
+    Portfolio p2 = new Portfolio(2, 2);
+    p2.positions = null;
+    region.put(2, p2);
+
+    Portfolio p3 = new Portfolio(3, 3);
+    p3.positions = new HashMap();
+    p3.positions.put("IBM", "something");
+    p3.positions.put("SUN", null);
+    region.put(3, p3);
+    region.put(3, p3);
+    
+    SelectResults result = (SelectResults) qs.newQuery("select * from /portfolio p where
p.positions['SUN'] = null").execute();
+    assertEquals(1, result.size());
+  }
+
+  @Test
+  public void testNullMapValuesInIndexOnLocalRegionForMap() throws Exception{
+    IndexManager.TEST_RANGEINDEX_ONLY = true;
+    region = CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+    qs = CacheUtils.getQueryService();
+    keyIndex1 = (IndexProtocol) qs.createIndex(INDEX_NAME, "positions[*]", "/portfolio ");
+
+    Portfolio p = new Portfolio(1, 1);
+    p.positions = new HashMap();
+    region.put(1, p);
+
+    Portfolio p2 = new Portfolio(2, 2);
+    p2.positions = null;
+    region.put(2, p2);
+
+    Portfolio p3 = new Portfolio(3, 3);
+    p3.positions = new HashMap();
+    p3.positions.put("SUN", null);
+    region.put(3, p3);
     
+    SelectResults result = (SelectResults) qs.newQuery("select * from /portfolio p where
p.positions['SUN'] = null").execute();
+    assertEquals(1, result.size());
   }
 
   /**


Mime
View raw message