You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2015/08/14 22:40:31 UTC
[42/50] [abbrv] incubator-geode git commit: GEODE-105: Null value in
Map causes NPE with Map Indexes
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 <jh...@pivotal.io>
Authored: Tue Aug 11 14:07:01 2015 -0700
Committer: Jason Huynh <jh...@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());
}
/**