Skip to content

Commit

Permalink
GEODE-9004: Align results of queries with and without map indexes (ap…
Browse files Browse the repository at this point in the history 
…ache#6279)

* GEODE-9004: Align results of queries with and without map indexes

Queries in which map fields are involved
in the condition, sometimes do not return the same entries
if map indexes are used, compared to when map indexes
are not used.

Differences were found when the condition on the
map field was of type '!=' and also when the condition
on the map field was comparing the value with null.

Example1:
Running a query with the following condition:
positions['SUN'] = null

in a region with the following entries:
entry1.positions=null
entry2.positions={'a'=>'b'}
entry3.positions={'SUN'=>null}

- will return entry1 and entry3 if there are no
  indexes defined.
- will return no entries if the following index is defined:
  positions['SUN','c']
- will return entry3 if the following index is defined:
  positions[*]

Example2:
Running a query with the following condition:
positions['SUN'] != '3'

in a region with the following entries:
entry1.positions=null
entry2.positions={'a'=>'b'}
entry3.positions={'SUN'=>'4'}
entry4.positions={'SUN'=>'3'}
entry5.positions={'SUN'=>null}

- will return all entries except for entry4 if:
  there are no indexes defined.
- will return entry3 if the following index is defined:
  positions['SUN','c']
- will return entry3 and entry5 if the following index is defined:
  positions[*]

In order to have the same results for these
queries no matter if indexes are used,
the following changes have been made:

- When using compact map indexes or map indexes
an index entry will be created for every entry
added to the region even if the entry does not
contain the indexed map (the map is null) or
it does not contain any of the indexed keys.
The above will not apply to indexes of type
"allkeys" (e.g. positions[*]).

- As a consequence of the statement above
about indexes of type "allkeys", when the index
configured is of "allkeys" type and the query
is of type "!=" (e.g. positions['SUN'] != "3"
or the query is comparing against a null value
(e.g. positions['SUN'] = null) the index will
not be used.
This is a limitation of this solution
as allowing the index to be used in this case
would require bigger and more complex changes that
have not been considered in this solution.

* GEODE-9004: Fix stats for number of keys in compact map indexes

* GEODE-9004: Fix failing test cases polluted by other test cases changing a System Property

* GEODE-9004: Changes after review and some refactorings

* GEODE-9004: Add documentation changes

* GEODE-9004: Small change in doc. after review
  • Loading branch information
@albertogpz
albertogpz authored May 7, 2021
1 parent 95c27da commit 884d399
Show file tree
Hide file tree
Showing 13 changed files with 581 additions and 122 deletions.
16 changes: 12 additions & 4 deletions ...tionTest/java/org/apache/geode/cache/query/internal/index/CompactRangeIndexJUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class CompactRangeIndexJUnitTest {

@Before
public void setUp() {
System.setProperty("index_elemarray_threshold", "3");
IndexManager.INDEX_ELEMARRAY_THRESHOLD_FOR_TESTING = 3;
utils = new QueryTestUtils();
Properties props = new Properties();
props.setProperty(MCAST_PORT, "0");
Expand All @@ -68,7 +68,6 @@ public void setUp() {

@Test
public void testCompactRangeIndex() throws Exception {
System.setProperty("index_elemarray_threshold", "3");
index = utils.createIndex("type", "\"type\"", SEPARATOR + "exampleRegion");
putValues(9);
isUsingIndexElemArray("type1");
Expand Down Expand Up @@ -289,7 +288,6 @@ public void run() {
assertEquals("incorrect number of entries in collection", 3, count);
} finally {
DefaultQuery.testHook = null;
System.setProperty("index_elemarray_threshold", "100");
}
}

Expand Down Expand Up @@ -477,7 +475,7 @@ public void testNullAndUndefinedValuesForMapKeyInCompactRangeIndex() throws Exce
p4.positions.put("SUN", null);
region.put("KEY-" + 4, p4);

// execute query and check result size
// execute query for null value and check result size
QueryService qs = utils.getCache().getQueryService();
SelectResults<Object> results = UncheckedUtils.uncheckedCast(qs
.newQuery(
Expand All @@ -486,6 +484,15 @@ public void testNullAndUndefinedValuesForMapKeyInCompactRangeIndex() throws Exce
assertThat(results.size()).isEqualTo(2);
assertThat(results.contains(p2)).isTrue();
assertThat(results.contains(p4)).isTrue();

// execute query for not null value and check result size
results = UncheckedUtils.uncheckedCast(qs
.newQuery(
"Select * from " + SEPARATOR + "exampleRegion r where r.positions['SUN'] != null")
.execute());
assertThat(results.size()).isEqualTo(2);
assertThat(results.contains(p1)).isTrue();
assertThat(results.contains(p3)).isTrue();
}

private void putValues(int num) {
Expand Down Expand Up @@ -620,6 +627,7 @@ private int executeRangeQueryWithoutDistinct(int expResults) {

@After
public void tearDown() throws Exception {
IndexManager.INDEX_ELEMARRAY_THRESHOLD_FOR_TESTING = -1;
utils.closeCache();
}

Expand Down
4 changes: 2 additions & 2 deletions ...ationTest/java/org/apache/geode/cache/query/internal/index/IndexMaintenanceJUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ public void testMapKeyIndexMaintenanceForNonCompactTypeSpecificKeysIndex() throw
// Test index maintenance
// addition of new Portfolio object
Map<Object, AbstractIndex> indxMap = mri.getRangeIndexHolderForTesting();
assertEquals(indxMap.size(), 3);
assertEquals(indxMap.size(), 4);
for (int j = 1; j <= 3; ++j) {
assertTrue(indxMap.containsKey("key" + j));
RangeIndex rng = (RangeIndex) indxMap.get("key" + j);
Expand Down Expand Up @@ -717,7 +717,7 @@ public void testMapKeyIndexMaintenanceForNonCompactTypeSpecificKeysIndex() throw
mkid.maap.put("key" + j, "val" + j);
}
testRgn.put(ID, mkid);
assertEquals(indxMap.size(), 3);
assertEquals(indxMap.size(), 4);
for (int j = 1; j <= 3; ++j) {
assertTrue(indxMap.containsKey("key" + j));
RangeIndex rng = (RangeIndex) indxMap.get("key" + j);
Expand Down
55 changes: 27 additions & 28 deletions ...rationTest/java/org/apache/geode/cache/query/internal/index/IndexStatisticsJUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,18 +346,17 @@ public void testStatsForCompactMapRangeIndex() throws Exception {

assertEquals(2, keyIndexStats.getNumberOfMapIndexKeys());
assertEquals(100, keyIndexStats.getNumberOfKeys());
assertEquals(100, keyIndexStats.getNumberOfKeys());
assertEquals(100, keyIndexStats.getNumberOfValues());
assertEquals(100, keyIndexStats.getNumUpdates());
assertEquals(200, keyIndexStats.getNumberOfValues());
assertEquals(200, keyIndexStats.getNumUpdates());

for (int i = 0; i < 100; i++) {
region.put(Integer.toString(i), new Portfolio(i, i));
}

assertEquals(2, keyIndexStats.getNumberOfMapIndexKeys());
assertEquals(100, keyIndexStats.getNumberOfKeys());
assertEquals(100, keyIndexStats.getNumberOfValues());
assertEquals(200, keyIndexStats.getNumUpdates());
assertEquals(200, keyIndexStats.getNumberOfValues());
assertEquals(400, keyIndexStats.getNumUpdates());

String queryStr =
"select * from " + SEPARATOR
Expand All @@ -378,23 +377,23 @@ public void testStatsForCompactMapRangeIndex() throws Exception {

assertEquals(2, keyIndexStats.getNumberOfMapIndexKeys());
assertEquals(50, keyIndexStats.getNumberOfKeys());
assertEquals(50, keyIndexStats.getNumberOfValues());
assertEquals(250, keyIndexStats.getNumUpdates());
assertEquals(100, keyIndexStats.getNumberOfValues());
assertEquals(500, keyIndexStats.getNumUpdates());

for (int i = 0; i < 50; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(2, keyIndexStats.getNumberOfMapIndexKeys());
assertEquals(50, keyIndexStats.getNumberOfKeys());
assertEquals(50, keyIndexStats.getNumberOfValues());
assertEquals(250, keyIndexStats.getNumUpdates());
assertEquals(100, keyIndexStats.getNumberOfValues());
assertEquals(500, keyIndexStats.getNumUpdates());

for (int i = 50; i < 100; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(300, keyIndexStats.getNumUpdates());
assertEquals(600, keyIndexStats.getNumUpdates());

assertEquals(2, keyIndexStats.getNumberOfMapIndexKeys());
assertEquals(0, keyIndexStats.getNumberOfKeys());
Expand Down Expand Up @@ -424,8 +423,8 @@ public void testStatsForMapRangeIndex() throws Exception {

assertEquals(2, mapIndexStats.getNumberOfMapIndexKeys());
assertEquals(100, mapIndexStats.getNumberOfKeys());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(100, mapIndexStats.getNumUpdates());
assertEquals(200, mapIndexStats.getNumberOfValues());
assertEquals(200, mapIndexStats.getNumUpdates());


Position.cnt = 0;
Expand All @@ -435,8 +434,8 @@ public void testStatsForMapRangeIndex() throws Exception {

assertEquals(2, mapIndexStats.getNumberOfMapIndexKeys());
assertEquals(100, mapIndexStats.getNumberOfKeys());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(200, mapIndexStats.getNumUpdates());
assertEquals(200, mapIndexStats.getNumberOfValues());
assertEquals(400, mapIndexStats.getNumUpdates());
String queryStr =
"select * from " + SEPARATOR
+ "portfolio where positions['DELL'] != NULL OR positions['YHOO'] != NULL";
Expand All @@ -457,23 +456,23 @@ public void testStatsForMapRangeIndex() throws Exception {

assertEquals(2, mapIndexStats.getNumberOfMapIndexKeys());
assertEquals(50, mapIndexStats.getNumberOfKeys());
assertEquals(50, mapIndexStats.getNumberOfValues());
assertEquals(300, mapIndexStats.getNumUpdates());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(600, mapIndexStats.getNumUpdates());

for (int i = 0; i < 50; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(2, mapIndexStats.getNumberOfMapIndexKeys());
assertEquals(50, mapIndexStats.getNumberOfKeys());
assertEquals(50, mapIndexStats.getNumberOfValues());
assertEquals(300, mapIndexStats.getNumUpdates());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(600, mapIndexStats.getNumUpdates());

for (int i = 50; i < 100; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(400, mapIndexStats.getNumUpdates());
assertEquals(800, mapIndexStats.getNumUpdates());

assertEquals(2, mapIndexStats.getNumberOfMapIndexKeys());
assertEquals(0, mapIndexStats.getNumberOfKeys());
Expand Down Expand Up @@ -667,8 +666,8 @@ public void testStatsForMapRangeIndexBeforeRegionCreation() throws Exception {
IndexStatistics mapIndexStats = keyIndex3.getStatistics();

assertEquals(100, mapIndexStats.getNumberOfKeys());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(100, mapIndexStats.getNumUpdates());
assertEquals(200, mapIndexStats.getNumberOfValues());
assertEquals(200, mapIndexStats.getNumUpdates());


Position.cnt = 0;
Expand All @@ -677,8 +676,8 @@ public void testStatsForMapRangeIndexBeforeRegionCreation() throws Exception {
}

assertEquals(100, mapIndexStats.getNumberOfKeys());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(200, mapIndexStats.getNumUpdates());
assertEquals(200, mapIndexStats.getNumberOfValues());
assertEquals(400, mapIndexStats.getNumUpdates());

String queryStr =
"select * from " + SEPARATOR
Expand All @@ -696,23 +695,23 @@ public void testStatsForMapRangeIndexBeforeRegionCreation() throws Exception {
}

assertEquals(50, mapIndexStats.getNumberOfKeys());
assertEquals(50, mapIndexStats.getNumberOfValues());
assertEquals(300, mapIndexStats.getNumUpdates());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(600, mapIndexStats.getNumUpdates());


for (int i = 0; i < 50; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(50, mapIndexStats.getNumberOfKeys());
assertEquals(50, mapIndexStats.getNumberOfValues());
assertEquals(300, mapIndexStats.getNumUpdates());
assertEquals(100, mapIndexStats.getNumberOfValues());
assertEquals(600, mapIndexStats.getNumUpdates());

for (int i = 50; i < 100; i++) {
region.destroy(Integer.toString(i));
}

assertEquals(400, mapIndexStats.getNumUpdates());
assertEquals(800, mapIndexStats.getNumUpdates());

assertEquals(0, mapIndexStats.getNumberOfKeys());

Expand Down
Loading

0 comments on commit 884d399

Please sign in to comment.