diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexIndexMapJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexIndexMapJUnitTest.java index a82aec42be8f..389127568e10 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexIndexMapJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexIndexMapJUnitTest.java @@ -20,6 +20,7 @@ import java.text.ParseException; import java.util.HashMap; +import java.util.Iterator; import org.junit.After; import org.junit.Before; @@ -115,6 +116,45 @@ public void testMultipleSecondLevelMatches() throws Exception { IndexManager.IS_TEST_EXPANSION = oldTestExpansionValue; } + @Test + public void testDescendingRangeQueryWithLimitReturnsSameOrderForMapIndexStore() throws Exception { + String queryString = "Select * from " + SEPARATOR + + "portfolios p where p.ID >= 2 and p.ID <= 7 order by p.ID desc limit 3"; + + Cache cache = CacheUtils.getCache(); + QueryService queryService = cache.getQueryService(); + int numEntries = 20; + boolean oldTestLDMValue = IndexManager.IS_TEST_LDM; + boolean oldTestExpansionValue = IndexManager.IS_TEST_EXPANSION; + + try { + IndexManager.IS_TEST_LDM = false; + IndexManager.IS_TEST_EXPANSION = false; + Region region = createReplicatedRegion("portfolios"); + createPortfolios(region, numEntries); + + queryService.createIndex("IDIndex", "p.ID", SEPARATOR + "portfolios p"); + SelectResults memResults = (SelectResults) queryService.newQuery(queryString).execute(); + validatePortfolioOrder(memResults, 7, 6, 5); + queryService.removeIndexes(); + region.destroyRegion(); + + IndexManager.IS_TEST_LDM = true; + IndexManager.IS_TEST_EXPANSION = true; + region = createLDMRegion("portfolios"); + createPortfolios(region, numEntries); + + queryService.createIndex("IDIndex", "p.ID", SEPARATOR + "portfolios p"); + SelectResults ldmResults = (SelectResults) queryService.newQuery(queryString).execute(); + validatePortfolioOrder(ldmResults, 7, 6, 5); + queryService.removeIndexes(); + region.destroyRegion(); + } finally { + IndexManager.IS_TEST_LDM = oldTestLDMValue; + IndexManager.IS_TEST_EXPANSION = oldTestExpansionValue; + } + } + // executes queries against both no index and ldm index // compares size counts of both and compares results private void testIndexAndQuery(String indexExpression, String regionPath, String queryString) @@ -187,6 +227,16 @@ private Region createReplicatedRegion(String regionName) throws ParseException { return cache.createRegion(regionName, regionAttributes); } + private void validatePortfolioOrder(SelectResults results, int... expectedIds) { + assertEquals("Unexpected result size", expectedIds.length, results.size()); + Iterator iterator = results.iterator(); + for (int expectedId : expectedIds) { + Portfolio portfolio = (Portfolio) iterator.next(); + assertEquals("Unexpected portfolio order", expectedId, portfolio.getID()); + } + assertEquals("Returned more results than expected", false, iterator.hasNext()); + } + private void createPortfolios(Region region, int num) { for (int i = 0; i < num; i++) { Portfolio p = new Portfolio(i); diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapIndexStoreJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapIndexStoreJUnitTest.java index d7587d3b609f..58163e0866da 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapIndexStoreJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapIndexStoreJUnitTest.java @@ -163,6 +163,23 @@ private void validateDescendingIterator(CloseableIterator iterator, int reverseS } } + private void validateDescendingRangeIterator(CloseableIterator iterator, + int reverseStart, int reverseEndInclusive) { + try { + for (int i = reverseStart; i >= reverseEndInclusive; i--) { + IndexStoreEntry ise = iterator.next(); + if (Integer.parseInt((String) ise.getDeserializedKey()) != i) { + fail("descending range iterator did not return the expected reverse order"); + } + } + assertEquals("descending range iterator returned extra values", false, iterator.hasNext()); + } finally { + if (iterator != null) { + iterator.close(); + } + } + } + /** * Helper method to test index storage iterators */ @@ -283,6 +300,26 @@ public void testStartExclusiveEndInclusive() throws IMQException { helpTestStartAndEndIterator(region, startValue, true, endValue, false, numValues - 1); } + @Test + public void testDescendingRangeIteratorReturnsReverseOrder() throws IMQException { + addValues(region, numValues); + + CloseableIterator iterator = + indexDataStructure.descendingIterator("2", true, "7", true, null); + + validateDescendingRangeIterator(iterator, 7, 2); + } + + @Test + public void testDescendingRangeIteratorHonorsExclusiveBounds() throws IMQException { + addValues(region, numValues); + + CloseableIterator iterator = + indexDataStructure.descendingIterator("2", false, "7", false, null); + + validateDescendingRangeIterator(iterator, 6, 3); + } + private class IndexRegionTestEntry implements IndexStoreEntry { diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MapIndexStore.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MapIndexStore.java index 0c7e8abac2ea..895884b449be 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MapIndexStore.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MapIndexStore.java @@ -145,9 +145,12 @@ public CloseableIterator descendingIterator(Collection keysToRe @Override public CloseableIterator descendingIterator(Object start, boolean startInclusive, Object end, boolean endInclusive, Collection keysToRemove) { - // @todo change to descending once it is supported - return new MapIndexStoreIterator(indexMap.iterator(start, startInclusive, end, endInclusive), - keysToRemove, indexOnValues, indexOnRegionKeys); + if (start == null) { + return descendingIterator(end, endInclusive, keysToRemove); + } + return new MapIndexStoreIterator( + indexMap.descendingIterator(start, startInclusive, end, endInclusive), keysToRemove, + indexOnValues, indexOnRegionKeys); } @Override diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/IndexMap.java b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/IndexMap.java index 6767f22b9ada..ffd1645a2217 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/IndexMap.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/IndexMap.java @@ -95,6 +95,12 @@ CloseableIterator iterator(Object start, boolean startInclusive, Obj */ CloseableIterator descendingIterator(); + /** + * Return all of the IndexEntries in the range between start and end, in descending order. + */ + CloseableIterator descendingIterator(Object start, boolean startInclusive, + Object end, boolean endInclusive); + /** * Return all of the region keys from start to end. */ diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/mock/IndexMapImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/mock/IndexMapImpl.java index 67b5bfc392b2..27aa562f9853 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/mock/IndexMapImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/persistence/query/mock/IndexMapImpl.java @@ -106,6 +106,16 @@ public CloseableIterator descendingIterator() { return new Itr(map.descendingMap().entrySet().iterator()); } + @Override + public CloseableIterator descendingIterator(Object start, boolean startInclusive, + Object end, boolean endInclusive) { + byte[] startBytes = startInclusive ? ByteComparator.MIN_BYTES : ByteComparator.MAX_BYTES; + byte[] endBytes = endInclusive ? ByteComparator.MAX_BYTES : ByteComparator.MIN_BYTES; + return new Itr(map + .subMap(new Pair(start, startBytes), startInclusive, new Pair(end, endBytes), endInclusive) + .descendingMap().entrySet().iterator()); + } + @Override public CloseableIterator keyIterator(Object start, boolean startInclusive, Object end, boolean endInclusive) {