DonalEvans commented on a change in pull request #6200:
URL: https://github.com/apache/geode/pull/6200#discussion_r604441480
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,306 @@ public void
testNullMapValuesInIndexOnLocalRegionForCompactMap() throws Exceptio
assertEquals(1, result.size());
}
+ @Test
+ public void testQueriesForValueInMapFieldWithoutIndex() throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+ testQueriesForValueInMapField(region, qs);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithCompactMapIndexWithOneKey()
throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 = qs.createIndex(INDEX_NAME, "positions['SUN']", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactRangeIndex) keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes in region entries for the 'SUN' key:
+ // ("nothing", "more", null) + 1 for any entry that does not have
+ // a value for the "SUN" key (UNDEFINED)
+ assertThat(keys).isEqualTo(4);
+ // mapIndexKeys must be zero because the index used is a range index and
not a map index
+ assertThat(mapIndexKeys).isEqualTo(0);
+ // The number of values must be equal to the number of region entries
+ assertThat(values).isEqualTo(7);
+ // The index must be used in every query
+ assertThat(uses).isEqualTo(4);
+ }
+
+ @Test
+ public void
testQueriesForValueInMapFieldWithCompactMapIndexWithSeveralKeys() throws
Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 =
+ qs.createIndex(INDEX_NAME, "positions['SUN', 'ERICSSON']", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactMapRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactMapRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes for the 'SUN' key (null, "nothing", "more") plus
+ // the number of different values the positions map takes for the
'ERICSSON' key
+ // ("hey") for each entry in the region.
+ assertThat(keys).isEqualTo(4);
+ // The number of mapIndexKeys must be equal to the number of keys
+ // in the index that appear in region entries:
+ // 'SUN', 'ERICSSON'
+ assertThat(mapIndexKeys).isEqualTo(2);
+ // The number of values must be equal to the number of values the
+ // positions map takes for each key indexed in the map
+ // for each entry in the region:
+ // null, "nothing", "more", "hey", "more"
+ assertThat(values).isEqualTo(5);
+ // The index must not be used in queries with "!="
+ assertThat(uses).isEqualTo(2);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithCompactMapIndexWithStar()
throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 = qs.createIndex(INDEX_NAME, "positions[*]", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactMapRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactMapRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes for each key (null not included)
+ // for each entry in the region:
+ // "something", null, "nothing", "more", "hey", "tip"
+ assertThat(keys).isEqualTo(7);
Review comment:
This value is now in disagreement with the comment above, as the list of
values includes only 6 values. Either the explanation in the comment needs to
be changed, or the value being returned here is incorrect and there is a bug
somewhere. Also, it is a little confusing in terms of what "null not included"
is referring to, since the list of values includes null.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,306 @@ public void
testNullMapValuesInIndexOnLocalRegionForCompactMap() throws Exceptio
assertEquals(1, result.size());
}
+ @Test
+ public void testQueriesForValueInMapFieldWithoutIndex() throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+ testQueriesForValueInMapField(region, qs);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithCompactMapIndexWithOneKey()
throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 = qs.createIndex(INDEX_NAME, "positions['SUN']", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactRangeIndex) keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes in region entries for the 'SUN' key:
+ // ("nothing", "more", null) + 1 for any entry that does not have
+ // a value for the "SUN" key (UNDEFINED)
+ assertThat(keys).isEqualTo(4);
+ // mapIndexKeys must be zero because the index used is a range index and
not a map index
+ assertThat(mapIndexKeys).isEqualTo(0);
+ // The number of values must be equal to the number of region entries
+ assertThat(values).isEqualTo(7);
+ // The index must be used in every query
+ assertThat(uses).isEqualTo(4);
+ }
+
+ @Test
+ public void
testQueriesForValueInMapFieldWithCompactMapIndexWithSeveralKeys() throws
Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 =
+ qs.createIndex(INDEX_NAME, "positions['SUN', 'ERICSSON']", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactMapRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactMapRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes for the 'SUN' key (null, "nothing", "more") plus
+ // the number of different values the positions map takes for the
'ERICSSON' key
+ // ("hey") for each entry in the region.
+ assertThat(keys).isEqualTo(4);
+ // The number of mapIndexKeys must be equal to the number of keys
+ // in the index that appear in region entries:
+ // 'SUN', 'ERICSSON'
+ assertThat(mapIndexKeys).isEqualTo(2);
+ // The number of values must be equal to the number of values the
+ // positions map takes for each key indexed in the map
+ // for each entry in the region:
+ // null, "nothing", "more", "hey", "more"
+ assertThat(values).isEqualTo(5);
+ // The index must not be used in queries with "!="
+ assertThat(uses).isEqualTo(2);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithCompactMapIndexWithStar()
throws Exception {
+ region =
+
CacheUtils.getCache().createRegionFactory(RegionShortcut.REPLICATE).create("portfolio");
+ qs = CacheUtils.getQueryService();
+
+ keyIndex1 = qs.createIndex(INDEX_NAME, "positions[*]", SEPARATOR +
"portfolio ");
+ assertThat(keyIndex1).isInstanceOf(CompactMapRangeIndex.class);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ long uses =
+ ((CompactMapRangeIndex) keyIndex1).internalIndexStats.getTotalUses();
+
+ // The number of keys must be equal to the number of different values the
+ // positions map takes for each key (null not included)
+ // for each entry in the region:
+ // "something", null, "nothing", "more", "hey", "tip"
+ assertThat(keys).isEqualTo(7);
+ // The number of mapIndexKeys must be equal to the number of different keys
+ // that appear in entries of the region:
+ // "IBM", "ERICSSON", "HP", "SUN", null
+ assertThat(mapIndexKeys).isEqualTo(5);
+ // The number of values must be equal to the number of values the
+ // positions map takes for each key (null not included)
+ // for each entry in the region:
+ // "something", null, "nothing", "more", "empty", "hey", "more", "tip"
+ assertThat(values).isEqualTo(8);
Review comment:
This comment is a little confusing in terms of what "null not included"
is referring to, since the list of values includes null.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/partitioned/PRIndexStatisticsJUnitTest.java
##########
@@ -273,8 +273,8 @@ public void testStatsForCompactMapRangeIndex() throws
Exception {
query.execute();
}
- // Both RangeIndex should be used
- assertEquals(100 /* Execution time */, keyIndexStats.getTotalUses());
+ // Index should not be used
Review comment:
Could this comment (and others in this class) be expanded slightly to
explain why the index should not be used here? I am assuming it's because the
query uses `!=`, but it would be good to make it explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
