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 <n...@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" <commits@geode.apache.org>'].