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/feature/GEODE-77 Commit: fcd03406c13d8f22b0222b337c6309ab94fce69c Parents: 18bbd9b Author: Jason Huynh <jhu...@pivotal.io> Authored: Tue Aug 11 14:07:01 2015 -0700 Committer: Jason Huynh <jhu...@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()); } /**