You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/03/26 23:09:35 UTC
[GitHub] [geode] DonalEvans commented on a change in pull request #6200: GEODE-9004: Fix issues in queries targeting a Map field
DonalEvans commented on a change in pull request #6200:
URL: https://github.com/apache/geode/pull/6200#discussion_r602562611
##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,167 @@ 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 testQueriesForValueInMapFieldWithMapIndexWithOneKey() 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();
+
+ assertThat(keys).isEqualTo(4);
+ assertThat(mapIndexKeys).isEqualTo(0);
+ assertThat(values).isEqualTo(7);
+ assertThat(uses).isEqualTo(4);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithMapIndexWithSeveralKeys() 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();
+
+ assertThat(keys).isEqualTo(3);
+ assertThat(mapIndexKeys).isEqualTo(1);
+ assertThat(values).isEqualTo(3);
+ assertThat(uses).isEqualTo(2);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithMapIndexWithStar() 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();
+ assertThat(keys).isEqualTo(5);
+ assertThat(mapIndexKeys).isEqualTo(4);
+ assertThat(values).isEqualTo(5);
+ assertThat(uses).isEqualTo(2);
+ }
+
+ public void testQueriesForValueInMapField(Region<Object, Object> region, QueryService qs)
+ throws Exception {
+ // Empty map
+ Portfolio p = new Portfolio(1, 1);
+ p.positions = new HashMap<>();
+ region.put(1, p);
+
+ // Map is null
+ Portfolio p2 = new Portfolio(2, 2);
+ p2.positions = null;
+ region.put(2, p2);
+
+ // Map with null value for "SUN" key
+ Portfolio p3 = new Portfolio(3, 3);
+ p3.positions = new HashMap<>();
+ p3.positions.put("IBM", "something");
+ p3.positions.put("SUN", null);
+ region.put(3, p3);
+
+ // Map with not null value for "SUN" key
+ Portfolio p4 = new Portfolio(4, 4);
+ p4.positions = new HashMap<>();
+ p4.positions.put("SUN", "nothing");
+ region.put(4, p4);
+
+ // Map with null key
+ Portfolio p5 = new Portfolio(5, 5);
+ p5.positions = new HashMap<>();
+ p5.positions.put("SUN", "more");
+ // The next one causes trouble with gfsh as json cannot show maps with null keys
+ p5.positions.put(null, "empty");
+ region.put(5, p5);
+
+ // One more with map without the "SUN" key
+ Portfolio p6 = new Portfolio(6, 6);
+ p6.positions = new HashMap<>();
+ p6.positions.put("ERIC", "hey");
+ region.put(6, p6);
+
+ // One more with null map
Review comment:
Is it necessary to have two region entries with null maps? p2 also has the same null `positions` field.
##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,167 @@ 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 testQueriesForValueInMapFieldWithMapIndexWithOneKey() 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();
+
+ assertThat(keys).isEqualTo(4);
+ assertThat(mapIndexKeys).isEqualTo(0);
+ assertThat(values).isEqualTo(7);
+ assertThat(uses).isEqualTo(4);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithMapIndexWithSeveralKeys() 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();
+
+ assertThat(keys).isEqualTo(3);
+ assertThat(mapIndexKeys).isEqualTo(1);
+ assertThat(values).isEqualTo(3);
+ assertThat(uses).isEqualTo(2);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithMapIndexWithStar() 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();
+ assertThat(keys).isEqualTo(5);
+ assertThat(mapIndexKeys).isEqualTo(4);
+ assertThat(values).isEqualTo(5);
+ assertThat(uses).isEqualTo(2);
Review comment:
See above comment about it being unclear where these values are coming from.
##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,167 @@ 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 testQueriesForValueInMapFieldWithMapIndexWithOneKey() 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();
+
+ assertThat(keys).isEqualTo(4);
+ assertThat(mapIndexKeys).isEqualTo(0);
+ assertThat(values).isEqualTo(7);
+ assertThat(uses).isEqualTo(4);
Review comment:
It's not immediately obvious where these values are coming from. Woudl it be possible to add a comment explaining why they are what they are?
##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,167 @@ public void testNullMapValuesInIndexOnLocalRegionForCompactMap() throws Exceptio
assertEquals(1, result.size());
}
+ @Test
+ public void testQueriesForValueInMapFieldWithoutIndex() throws Exception {
+ region =
Review comment:
Multiple warnings in this file can be cleaned up by making `region` a `Region<Object, Object>` where it's declared.
##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +344,167 @@ 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 testQueriesForValueInMapFieldWithMapIndexWithOneKey() 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();
+
+ assertThat(keys).isEqualTo(4);
+ assertThat(mapIndexKeys).isEqualTo(0);
+ assertThat(values).isEqualTo(7);
+ assertThat(uses).isEqualTo(4);
+ }
+
+ @Test
+ public void testQueriesForValueInMapFieldWithMapIndexWithSeveralKeys() 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();
+
+ assertThat(keys).isEqualTo(3);
+ assertThat(mapIndexKeys).isEqualTo(1);
+ assertThat(values).isEqualTo(3);
+ assertThat(uses).isEqualTo(2);
Review comment:
See above comment about it being unclear where these values are coming from.
--
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:
users@infra.apache.org