Skip to content

Commit bf0c44c

Browse files
albertogpzalbertogpz
committed
GEODE-9602: QueryObserver improvements.
- Make QueryObserverHolder thread-safe - Allow having an observer per query by means of setting the observer in the query at the start of the execution. - Invoke beforeIterationEvaluation and afterIterationEvaluation callbacks when query is using indexes.
1 parent d0113fc commit bf0c44c

File tree

30 files changed

+261
-101
lines changed

30 files changed

+261
-101
lines changed

‎geode-core/src/integrationTest/java/org/apache/geode/cache/query/QueryServiceRegressionTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertTrue;
2222
import static org.junit.Assert.fail;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
2325

2426
import java.io.Serializable;
2527
import java.util.ArrayList;
@@ -339,7 +341,9 @@ public void testBugResultMismatch() throws Exception {
339341
SelectResults rs1 = (SelectResults) q1.execute();
340342
SelectResults rs2 = (SelectResults) q2.execute();
341343

342-
assertThatCode(() -> QueryUtils.union(rs1, rs2, null)).doesNotThrowAnyException();
344+
ExecutionContext context = mock(ExecutionContext.class);
345+
when(context.getObserver()).thenReturn(new QueryObserverAdapter());
346+
assertThatCode(() -> QueryUtils.union(rs1, rs2, context)).doesNotThrowAnyException();
343347
}
344348

345349
/**

‎geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/OrderByComparatorJUnitTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import static org.junit.Assert.assertFalse;
2121
import static org.junit.Assert.assertTrue;
2222
import static org.junit.Assert.fail;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
2325

2426
import java.lang.reflect.Field;
2527
import java.util.Collection;
@@ -209,6 +211,8 @@ public void testCompareThrowsClassCastException() throws Exception {
209211

210212
private OrderByComparator createComparator() throws Exception {
211213
StructTypeImpl objType = new StructTypeImpl();
212-
return new OrderByComparator(null, objType, null);
214+
ExecutionContext context = mock(ExecutionContext.class);
215+
when(context.getObserver()).thenReturn(new QueryObserverAdapter());
216+
return new OrderByComparator(null, objType, context);
213217
}
214218
}

‎geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/QueryObserverCallbacksTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.apache.geode.cache.query.data.Address;
4242
import org.apache.geode.cache.query.data.Employee;
4343
import org.apache.geode.cache.query.data.Portfolio;
44+
import org.apache.geode.cache.query.internal.index.IndexManager;
4445
import org.apache.geode.test.junit.categories.OQLQueryTest;
4546
import org.apache.geode.test.junit.rules.ServerStarterRule;
4647

@@ -85,6 +86,7 @@ public void setUp() throws Exception {
8586
@After
8687
public void tearDown() {
8788
QueryObserverHolder.reset();
89+
IndexManager.TEST_RANGEINDEX_ONLY = false;
8890
}
8991

9092
@SuppressWarnings("unchecked")
@@ -243,6 +245,59 @@ public void beforeAggregationsAndGroupByShouldBeCalledForAggregateFunctions() th
243245
verify(myQueryObserver, times(queries.size())).beforeAggregationsAndGroupBy(any());
244246
}
245247

248+
@Test
249+
public void testBeforeAndAfterIterationEvaluateNoWhere() throws Exception {
250+
Query query = queryService.newQuery(
251+
"select count(*) from " + SEPARATOR + "portfolio p");
252+
253+
query.execute();
254+
verify(myQueryObserver, times(0)).beforeIterationEvaluation(any(), any());
255+
verify(myQueryObserver, times(0)).afterIterationEvaluation(any());
256+
}
257+
258+
@Test
259+
public void testBeforeAndAfterIterationEvaluateWithoutIndex() throws Exception {
260+
Query query = queryService.newQuery(
261+
"select count(*) from " + SEPARATOR + "portfolio p where p.isActive = true ");
262+
263+
query.execute();
264+
verify(myQueryObserver, times(4)).beforeIterationEvaluation(any(), any());
265+
verify(myQueryObserver, times(4)).afterIterationEvaluation(any());
266+
}
267+
268+
@Test
269+
public void testBeforeAndAfterIterationEvaluateWithCompactRangeIndex() throws Exception {
270+
Query query = queryService.newQuery(
271+
"select count(*) from " + SEPARATOR + "portfolio p where p.isActive = true ");
272+
queryService.createIndex("isActiveIndex", "isActive", SEPARATOR + "portfolio");
273+
274+
query.execute();
275+
verify(myQueryObserver, times(2)).beforeIterationEvaluation(any(), any());
276+
verify(myQueryObserver, times(2)).afterIterationEvaluation(any());
277+
assertThat(myQueryObserver.dbIndx[2] == myQueryObserver.usedIndx)
278+
.as("Validate callback of Indexes").isTrue();
279+
assertThat(myQueryObserver.unusedIndx == myQueryObserver.dbIndx[0]
280+
|| myQueryObserver.unusedIndx == myQueryObserver.dbIndx[1])
281+
.as("Validate callback of Indexes").isTrue();
282+
}
283+
284+
@Test
285+
public void testBeforeAndAfterIterationEvaluateWithRangeIndex() throws Exception {
286+
IndexManager.TEST_RANGEINDEX_ONLY = true;
287+
Query query = queryService.newQuery(
288+
"select count(*) from " + SEPARATOR + "portfolio p where p.description = 'XXXX' ");
289+
queryService.createIndex("descriptionIndex", "description", SEPARATOR + "portfolio");
290+
291+
query.execute();
292+
verify(myQueryObserver, times(2)).beforeIterationEvaluation(any(), any());
293+
verify(myQueryObserver, times(2)).afterIterationEvaluation(any());
294+
assertThat(myQueryObserver.dbIndx[2] == myQueryObserver.usedIndx)
295+
.as("Validate callback of Indexes").isTrue();
296+
assertThat(myQueryObserver.unusedIndx == myQueryObserver.dbIndx[0]
297+
|| myQueryObserver.unusedIndx == myQueryObserver.dbIndx[1])
298+
.as("Validate callback of Indexes").isTrue();
299+
}
300+
246301
private static class MyQueryObserverImpl extends QueryObserverAdapter {
247302
private int j = 0;
248303
private Index usedIndx = null;

‎geode-core/src/integrationTest/java/org/apache/geode/cache/query/internal/QueryTraceJUnitTest.java‎

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package org.apache.geode.cache.query.internal;
1616

1717
import static org.apache.geode.cache.Region.SEPARATOR;
18+
import static org.assertj.core.api.Assertions.assertThat;
1819
import static org.junit.Assert.assertEquals;
1920
import static org.junit.Assert.assertFalse;
2021
import static org.junit.Assert.assertTrue;
@@ -60,10 +61,12 @@ public class QueryTraceJUnitTest {
6061
@Before
6162
public void setUp() throws Exception {
6263
CacheUtils.startCache();
64+
DefaultQuery.testHook = new BeforeQueryExecutionHook();
6365
}
6466

6567
@After
6668
public void tearDown() throws Exception {
69+
DefaultQuery.testHook = null;
6770
CacheUtils.closeCache();
6871
}
6972

@@ -104,7 +107,11 @@ public void testTraceOnPartitionedRegionWithTracePrefix() throws Exception {
104107
assertTrue(((DefaultQuery) query).isTraced());
105108

106109
SelectResults results = (SelectResults) query.execute();
107-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
110+
111+
// The IndexTrackingObserver should have been set
112+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
113+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
114+
108115
// The query should return all elements in region.
109116
assertEquals(region.size(), results.size());
110117
QueryObserverHolder.reset();
@@ -141,7 +148,11 @@ public void testTraceOnLocalRegionWithTracePrefix() throws Exception {
141148
assertTrue(((DefaultQuery) query).isTraced());
142149

143150
SelectResults results = (SelectResults) query.execute();
144-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
151+
152+
// The IndexTrackingObserver should have been set
153+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
154+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
155+
145156
// The query should return all elements in region.
146157
assertEquals(region.size(), results.size());
147158
QueryObserverHolder.reset();
@@ -183,7 +194,11 @@ public void testNegTraceOnPartitionedRegionWithTracePrefix() throws Exception {
183194
assertFalse(((DefaultQuery) query).isTraced());
184195

185196
SelectResults results = (SelectResults) query.execute();
186-
assertFalse(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
197+
198+
// The IndexTrackingObserver should not have been set
199+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
200+
assertThat(hook.getObserver()).isNotInstanceOf(IndexTrackingQueryObserver.class);
201+
187202
// The query should return all elements in region.
188203
assertEquals(region.size(), results.size());
189204
QueryObserverHolder.reset();
@@ -223,7 +238,11 @@ public void testNegTraceOnLocalRegionWithTracePrefix() throws Exception {
223238
assertFalse(((DefaultQuery) query).isTraced());
224239

225240
SelectResults results = (SelectResults) query.execute();
226-
assertFalse(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
241+
242+
// The IndexTrackingObserver should not have been set
243+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
244+
assertThat(hook.getObserver()).isNotInstanceOf(IndexTrackingQueryObserver.class);
245+
227246
// The query should return all elements in region.
228247
assertEquals(region.size(), results.size());
229248
QueryObserverHolder.reset();
@@ -262,7 +281,11 @@ public void testTraceOnPartitionedRegionWithTracePrefixNoComments() throws Excep
262281
assertTrue(((DefaultQuery) query).isTraced());
263282

264283
SelectResults results = (SelectResults) query.execute();
265-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
284+
285+
// The IndexTrackingObserver should have been set
286+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
287+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
288+
266289
// The query should return all elements in region.
267290
assertEquals(region.size(), results.size());
268291
QueryObserverHolder.reset();
@@ -296,8 +319,11 @@ public void testTraceOnLocalRegionWithTracePrefixNoComments() throws Exception {
296319
assertTrue(((DefaultQuery) query).isTraced());
297320

298321
SelectResults results = (SelectResults) query.execute();
299-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
300-
// The query should return all elements in region.
322+
323+
// The IndexTrackingObserver should have been set
324+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
325+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
326+
301327
assertEquals(region.size(), results.size());
302328
QueryObserverHolder.reset();
303329
}
@@ -331,7 +357,11 @@ public void testTraceOnPartitionedRegionWithSmallTracePrefixNoComments() throws
331357
assertTrue(((DefaultQuery) query).isTraced());
332358

333359
SelectResults results = (SelectResults) query.execute();
334-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
360+
361+
// The IndexTrackingObserver should have been set
362+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
363+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
364+
335365
// The query should return all elements in region.
336366
assertEquals(region.size(), results.size());
337367
QueryObserverHolder.reset();
@@ -366,7 +396,11 @@ public void testTraceOnLocalRegionWithSmallTracePrefixNoComments() throws Except
366396
assertTrue(((DefaultQuery) query).isTraced());
367397

368398
SelectResults results = (SelectResults) query.execute();
369-
assertTrue(QueryObserverHolder.getInstance() instanceof IndexTrackingQueryObserver);
399+
400+
// The IndexTrackingObserver should have been set
401+
BeforeQueryExecutionHook hook = (BeforeQueryExecutionHook) DefaultQuery.testHook;
402+
assertThat(hook.getObserver()).isInstanceOf(IndexTrackingQueryObserver.class);
403+
370404
// The query should return all elements in region.
371405
assertEquals(region.size(), results.size());
372406
QueryObserverHolder.reset();
@@ -438,4 +472,21 @@ public void testQueryFailLocalRegionWithSmallTracePrefixNoSpace() throws Excepti
438472
}
439473
}
440474

475+
private class BeforeQueryExecutionHook implements DefaultQuery.TestHook {
476+
private QueryObserver observer = null;
477+
478+
@Override
479+
public void doTestHook(final SPOTS spot, final DefaultQuery _ignored,
480+
final ExecutionContext executionContext) {
481+
switch (spot) {
482+
case BEFORE_QUERY_EXECUTION:
483+
observer = executionContext.getObserver();
484+
break;
485+
}
486+
}
487+
488+
public QueryObserver getObserver() {
489+
return observer;
490+
}
491+
}
441492
}

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/AbstractGroupOrRangeJunction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ private SelectResults auxIterateEvaluate(CompiledValue operand, ExecutionContext
383383
resultSet = QueryUtils.createResultCollection(context, elementType);
384384
}
385385

386-
QueryObserver observer = QueryObserverHolder.getInstance();
386+
QueryObserver observer = context.getObserver();
387387
try {
388388
observer.startIteration(intermediateResults, operand);
389389
Iterator iResultsIter = intermediateResults.iterator();

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/AllGroupJunction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ private SelectResults evaluateAndJunction(ExecutionContext context)
169169
iterOperandsToSend = new CompiledJunction(cv, this.operator);
170170
}
171171
}
172-
QueryObserver observer = QueryObserverHolder.getInstance();
172+
QueryObserver observer = context.getObserver();
173173
observer.beforeCartesianOfGroupJunctionsInAnAllGroupJunctionOfType_AND(results);
174174
resultsSet = QueryUtils.cartesian(results, itrsForResultFields, expansionList, finalList,
175175
context, iterOperandsToSend);

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledComparison.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ private SelectResults singleBaseCollectionFilterEvaluate(ExecutionContext contex
373373
// before the index lookup
374374
int op = reflectOnOperator(indexInfo._key());
375375
// actual index lookup
376-
QueryObserver observer = QueryObserverHolder.getInstance();
376+
QueryObserver observer = context.getObserver();
377377
List projAttrib = null;
378378
/*
379379
* Asif : First obtain the match level of index resultset. If the match level happens to be zero
@@ -535,7 +535,7 @@ private SelectResults doubleBaseCollectionFilterEvaluate(ExecutionContext contex
535535
// each of the
536536
// one dimensional array can be either genuine result object or StructImpl
537537
// object.
538-
QueryObserver observer = QueryObserverHolder.getInstance();
538+
QueryObserver observer = context.getObserver();
539539
context.cachePut(CompiledValue.INDEX_INFO, indxInfo);
540540
/*
541541
* Asif : If the independent Group of iterators passed is not null or the independent Group of

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledGroupBySelect.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private void mapOriginalOrderByColumns(ExecutionContext context)
155155
public SelectResults evaluate(ExecutionContext context) throws FunctionDomainException,
156156
TypeMismatchException, NameResolutionException, QueryInvocationTargetException {
157157
SelectResults selectResults = super.evaluate(context);
158-
QueryObserverHolder.getInstance().beforeAggregationsAndGroupBy(selectResults);
158+
context.getObserver().beforeAggregationsAndGroupBy(selectResults);
159159

160160
return this.applyAggregateAndGroupBy(selectResults, context);
161161
}

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIn.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ SelectResults singleBaseCollectionFilterEvaluate(ExecutionContext context,
524524
}
525525
}
526526

527-
QueryObserver observer = QueryObserverHolder.getInstance();
527+
QueryObserver observer = context.getObserver();
528528
try {
529529
Object evalColln = evaluateColln(context);
530530
observer.beforeIndexLookup(indexInfo._index, TOK_EQ, evalColln);

‎geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledJunction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ SelectResults auxIterateEvaluate(CompiledValue operand, ExecutionContext context
368368
}
369369

370370

371-
QueryObserver observer = QueryObserverHolder.getInstance();
371+
QueryObserver observer = context.getObserver();
372372
try {
373373
observer.startIteration(intermediateResults, operand);
374374
Iterator iResultsIter = intermediateResults.iterator();

0 commit comments

Comments
 (0)