You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nn...@apache.org on 2017/10/18 21:17:21 UTC

[geode] branch develop updated: GEODE-3708: Added a separate iterator for MemoryIndexStore which doesn't iterate over the values in the Map.

This is an automated email from the ASF dual-hosted git repository.

nnag pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 2e710e5  GEODE-3708: Added a separate iterator for MemoryIndexStore which doesn't iterate over the values in the Map.
2e710e5 is described below

commit 2e710e5d8ab11695c12f381f107398f2122f43ec
Author: nabarun <nn...@pivotal.io>
AuthorDate: Fri Oct 6 13:17:52 2017 -0700

    GEODE-3708: Added a separate iterator for MemoryIndexStore which doesn't iterate over the values in the Map.
    
    	* The new iterator MemoryIndexStoreKeyIterator will only iterate over the keys
    	* This is different from the previous iterator which iterates over the values too and sending out IndexStoreEntry pairs of the key and iterated value entry
    	* This resulted in duplicate results in join queries with OR clause as the same key was being repeatedly sent if the value associated with the key is a collection.
---
 .../query/internal/index/CompactRangeIndex.java    | 14 ++---
 .../query/internal/index/MemoryIndexStore.java     | 64 ++++++++++++++++++++++
 .../cache/query/JoinQueriesIntegrationTest.java    | 13 +++--
 3 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
index 7031532..2c19d9d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java
@@ -181,14 +181,16 @@ public class CompactRangeIndex extends AbstractIndex {
     long start = updateIndexUseStats();
     ((AbstractIndex) indx).updateIndexUseStats();
     List data = new ArrayList();
-    CloseableIterator<IndexStoreEntry> outer = null;
+    Iterator<IndexStoreEntry> outer = null;
     Iterator inner = null;
     try {
       // We will iterate over each of the index Map to obtain the keys
-      outer = indexStore.iterator(null);
+      outer = ((MemoryIndexStore) indexStore).getKeysIterator();
 
       if (indx instanceof CompactRangeIndex) {
-        inner = ((CompactRangeIndex) indx).getIndexStorage().iterator(null);
+        IndexStore indexStore = ((CompactRangeIndex) indx).getIndexStorage();
+        inner = ((MemoryIndexStore) indexStore).getKeysIterator();
+
       } else {
         inner = ((RangeIndex) indx).getValueToEntriesMap().entrySet().iterator();
       }
@@ -252,10 +254,8 @@ public class CompactRangeIndex extends AbstractIndex {
     } finally {
       ((AbstractIndex) indx).updateIndexUseEndStats(start);
       updateIndexUseEndStats(start);
-      if (outer != null) {
-        outer.close();
-      }
-      if (inner != null && indx instanceof CompactRangeIndex) {
+      if (inner != null && indx instanceof CompactRangeIndex
+          && inner instanceof CloseableIterator) {
         ((CloseableIterator<IndexStoreEntry>) inner).close();
       }
     }
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
index 8e298c9..91dbfc4 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java
@@ -420,6 +420,10 @@ public class MemoryIndexStore implements IndexStore {
         keysToRemove);
   }
 
+  public Iterator<IndexStoreEntry> getKeysIterator() {
+    return new MemoryIndexStoreKeyIterator(this.valueToEntriesMap);
+  }
+
   @Override
   public CloseableIterator<IndexStoreEntry> iterator(Collection keysToRemove) {
     return new MemoryIndexStoreIterator(this.valueToEntriesMap, null, keysToRemove);
@@ -560,6 +564,38 @@ public class MemoryIndexStore implements IndexStore {
     return numIndexKeys.get();
   }
 
+  private class MemoryIndexStoreKeyIterator implements Iterator<IndexStoreEntry> {
+
+    final private Map valuesToEntriesMap;
+    private Object currKey;
+    private Iterator<Map.Entry> mapIterator;
+
+    public MemoryIndexStoreKeyIterator(Map valuesToEntriesMap) {
+      this.valuesToEntriesMap = valuesToEntriesMap;
+    }
+
+    @Override
+    public boolean hasNext() {
+      if (mapIterator == null) {
+        mapIterator = this.valuesToEntriesMap.entrySet().iterator();
+      }
+      if (mapIterator.hasNext()) {
+        Map.Entry currentEntry = mapIterator.next();
+        currKey = currentEntry.getKey();
+        if (currKey == IndexManager.NULL || currKey == QueryService.UNDEFINED) {
+          return hasNext();
+        }
+        return currKey != null;
+      }
+      return false;
+    }
+
+    @Override
+    public MemoryIndexStoreKey next() {
+      return new MemoryIndexStoreKey(currKey);
+    }
+  }
+
   /**
    * A bi-directional iterator over the CSL. Iterates over the entries of CSL where entry is a
    * mapping (value -> Collection) as well as over the Collection.
@@ -704,6 +740,34 @@ public class MemoryIndexStore implements IndexStore {
     return sb.toString();
   }
 
+  class MemoryIndexStoreKey implements IndexStoreEntry {
+    private Object indexKey;
+
+    public MemoryIndexStoreKey(Object indexKey) {
+      this.indexKey = indexKey;
+    }
+
+    @Override
+
+    public Object getDeserializedKey() {
+      return indexKey;
+    }
+
+    @Override
+    public Object getDeserializedValue() {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Object getDeserializedRegionKey() {
+      throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public boolean isUpdateInProgress() {
+      throw new UnsupportedOperationException();
+    }
+  }
   /**
    * A wrapper over the entry in the CSL index map. It maps IndexKey -> RegionEntry
    */
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/JoinQueriesIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/JoinQueriesIntegrationTest.java
index f2d4ec0..f7d703b 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/JoinQueriesIntegrationTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/JoinQueriesIntegrationTest.java
@@ -27,9 +27,6 @@ import org.junit.runner.RunWith;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
-import org.apache.geode.cache.query.CacheUtils;
-import org.apache.geode.cache.query.QueryService;
-import org.apache.geode.cache.query.SelectResults;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
@@ -63,9 +60,13 @@ public class JoinQueriesIntegrationTest {
 
 
   private static Object[] getQueryStrings() {
-    return new Object[] {new Object[] {
-        "<trace>select STA.id as STACID, STA.pkid as STAacctNum, STC.id as STCCID, STC.pkid as STCacctNum from /region1 STA, /region2 STC where STA.pkid = 1 AND STA.joinId = STC.joinId AND STA.id = STC.id",
-        20},};
+    return new Object[] {
+        new Object[] {
+            "<trace>select STA.id as STACID, STA.pkid as STAacctNum, STC.id as STCCID, STC.pkid as STCacctNum from /region1 STA, /region2 STC where STA.pkid = 1 AND STA.joinId = STC.joinId AND STA.id = STC.id",
+            20},
+        new Object[] {
+            "<trace>select STA.id as STACID, STA.pkid as STAacctNum, STC.id as STCCID, STC.pkid as STCacctNum from /region1 STA, /region2 STC where STA.pkid = 1 AND STA.joinId = STC.joinId OR STA.id = STC.id",
+            22}};
   }
 
   @Test

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].