[
https://issues.apache.org/jira/browse/GEODE-9004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17297774#comment-17297774
]
ASF GitHub Bot commented on GEODE-9004:
---------------------------------------
DonalEvans commented on a change in pull request #6096:
URL: https://github.com/apache/geode/pull/6096#discussion_r589867114
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +341,155 @@ 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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(5, keys);
+ assertEquals(4, mapIndexKeys);
+ assertEquals(5, values);
+ }
+
+ public void testQueriesForValueInMapField(Region region, QueryService qs)
throws Exception {
Review comment:
The compiler warning here can be cleaned up by using `Region<Object,
Object>` here.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/partitioned/PRIndexStatisticsJUnitTest.java
##########
@@ -274,7 +274,7 @@ public void testStatsForCompactMapRangeIndex() throws
Exception {
}
// Both RangeIndex should be used
Review comment:
This comment seems incorrect now.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +341,155 @@ 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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(5, keys);
+ assertEquals(4, mapIndexKeys);
+ assertEquals(5, values);
+ }
+
+ public void testQueriesForValueInMapField(Region 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
+ Portfolio p7 = new Portfolio(7, 7);
+ p7.positions = null;
+ region.put(7, p7);
+
+ String query;
+ query = "select * from " + SEPARATOR + "portfolio p where
p.positions['SUN'] = null";
+ SelectResults result = (SelectResults) qs
Review comment:
Compiler warnings here and elsewhere in this method can be fixed by
using `SelectResults<Object>` and
`UncheckedUtils.uncheckedCast(qs.newQuery(query).execute());`
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +341,155 @@ 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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(5, keys);
+ assertEquals(4, mapIndexKeys);
+ assertEquals(5, values);
+ }
+
+ public void testQueriesForValueInMapField(Region region, QueryService qs)
throws Exception {
+ // Empty map
+ Portfolio p = new Portfolio(1, 1);
+ p.positions = new HashMap();
Review comment:
Compiler warnings can be cleaned up by using `HashMap<>()` throughout
this method.
##########
File path:
geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java
##########
@@ -341,6 +341,155 @@ 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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(3, keys);
+ assertEquals(1, mapIndexKeys);
+ assertEquals(3, values);
+ }
+
+ @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 ");
+ assertTrue(keyIndex1 instanceof CompactMapRangeIndex);
+ testQueriesForValueInMapField(region, qs);
+
+ long keys = ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfKeys();
+ long mapIndexKeys =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfMapIndexKeys();
+ long values =
+ ((CompactMapRangeIndex)
keyIndex1).internalIndexStats.getNumberOfValues();
+ assertEquals(5, keys);
+ assertEquals(4, mapIndexKeys);
+ assertEquals(5, values);
+ }
+
+ public void testQueriesForValueInMapField(Region 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
+ Portfolio p7 = new Portfolio(7, 7);
+ p7.positions = null;
+ region.put(7, p7);
+
+ String query;
+ query = "select * from " + SEPARATOR + "portfolio p where
p.positions['SUN'] = null";
+ SelectResults result = (SelectResults) qs
+ .newQuery(query)
+ .execute();
+ System.out.println("Query: " + query + ", result: " + result);
Review comment:
If output is strictly necessary in this method, it should be via a
logger, not `System.out`. Instead, however, it might be better to assert on the
query result to confirm that it matches what is expected beyond just having the
correct size, so that any failure will be self explanatory and not require this
debug logging.
----------------------------------------------------------------
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]
> Issues with queries targeting a map field
> -----------------------------------------
>
> Key: GEODE-9004
> URL: https://issues.apache.org/jira/browse/GEODE-9004
> Project: Geode
> Issue Type: Bug
> Components: querying
> Reporter: Alberto Gomez
> Assignee: Alberto Gomez
> Priority: Major
> Labels: pull-request-available
>
> When defining indexes on a map field several issues have been found:
> * If the index is defined for one specific key in the map, e.g.
> positions['SUN'], then an index entry is created for every entry, no matter
> if the entry contains the 'SUN' key in its map or not. This makes the index
> take a lot of memory (unnecessarily?).
> * If the index is specified for more than one key in the map or for all
> ('*'), then queries in which the "where" contains a != condition for the map,
> e.g. "p.positions['SUN'] != '3'" return less values than those returned when
> the query is run without the index.
> * If the index is specified for more than one key in the map or for all
> ('*'), then queries in which the "where" contains a "= null" condition for
> the map, e.g. "p.positions['SUN'] = null" return less values than those
> returned when the query is run without the index.
> * If the index is defined for one specific key in the map, e.g.
> positions['SUN'], queries in which the "where" contains a "!=" condition for
> the map or a " = null" condition sometimes return less values than those
> returned when the query is run without the index.
> Apart from the above, looking at the indexes documentation, it seems that Map
> indexes are only those indexes for which more than one key or '*' is
> specified for the Map. But if just one key is specified for the Map in the
> index, then the index is not a Map index but a range index. This should be
> clarified.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
