You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nn...@apache.org on 2018/11/16 20:33:49 UTC
[geode] branch develop updated: GEODE-6053: Parameterized Queries
fixed (#2859)
This is an automated email from the ASF dual-hosted git repository.
nnag pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new b622764 GEODE-6053: Parameterized Queries fixed (#2859)
b622764 is described below
commit b6227643a1041e264fdd72d24d5d2d8e1ead0fa9
Author: Nabarun Nag <na...@users.noreply.github.com>
AuthorDate: Fri Nov 16 12:33:40 2018 -0800
GEODE-6053: Parameterized Queries fixed (#2859)
* Parameterized queries were throwing UnsupportedOperationExceptions.
* This was because of trying to pre-compute values for join optimization.
* When the query was parameterized it is not possible to compute these values and hence these exceptions are thrown
* The fix was to prevent this computation when it is not needed or not possible.
---
...IndexUsageInNestedQueryWithParamsJUnitTest.java | 285 +++++++++++++++++++++
.../geode/cache/query/internal/CompiledID.java | 5 +-
.../query/internal/CompiledIndexOperation.java | 9 +
.../cache/query/internal/CompiledOperation.java | 9 +
.../geode/cache/query/internal/CompiledPath.java | 9 +
.../geode/cache/query/internal/CompiledValue.java | 4 +
.../geode/cache/query/internal/QueryUtils.java | 14 +-
.../AbstractCompiledValueTestJUnitTest.java | 112 ++++++++
8 files changed, 441 insertions(+), 6 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/IndexUsageInNestedQueryWithParamsJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/IndexUsageInNestedQueryWithParamsJUnitTest.java
new file mode 100644
index 0000000..212e142
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/IndexUsageInNestedQueryWithParamsJUnitTest.java
@@ -0,0 +1,285 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.query.functional;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.query.CacheUtils;
+import org.apache.geode.cache.query.Query;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.cache.query.SelectResults;
+import org.apache.geode.test.junit.categories.OQLIndexTest;
+
+@Category({OQLIndexTest.class})
+public class IndexUsageInNestedQueryWithParamsJUnitTest {
+ @Before
+ public void setUp() throws java.lang.Exception {
+ CacheUtils.startCache();
+ Region portfolioRegion =
+ CacheUtils.createRegion("portfolio", PortfolioForParameterizedQueries.class);
+ Region intBoundRegion = CacheUtils.createRegion("intBound", IntermediateBound.class);
+ QueryService queryService = CacheUtils.getQueryService();
+ queryService.defineIndex("portfolioIdIndex", "p.ID", "/portfolio p");
+ queryService.defineIndex("temporaryOriginalBoundIdIdx", "ib.originalId", "/intBound ib");
+ queryService.createDefinedIndexes();
+ PortfolioForParameterizedQueries p = new PortfolioForParameterizedQueries(1L);
+ p.setDataList(Arrays
+ .asList(new PortfolioData(Arrays.asList(new Bound(11L), new Bound(12L))),
+ new PortfolioData(Arrays.asList(new Bound(21L), new Bound(22L)))));
+ portfolioRegion.put(p.getID(), p);
+ intBoundRegion.put(91L, new IntermediateBound(91L, 12L, new Bound(91L)));
+ intBoundRegion.put(92L, new IntermediateBound(92L, 21L, new Bound(92L)));
+ }
+
+ @After
+ public void tearDown() throws java.lang.Exception {
+ CacheUtils.closeCache();
+ }
+
+ @Test
+ public void whenParameterizedQueryExecutedThenItShouldReturnCorrectResultsAndNotThrowException()
+ throws Exception {
+
+ QueryService queryService = CacheUtils.getQueryService();
+ Query query = queryService.newQuery(
+ "<TRACE>select l.ID from /portfolio p, p.dataList dl, ($1).getConcatenatedBoundsAndIntermediateBounds(dl) l WHERE p.ID IN SET(1L,2L)");
+ QueryParameterProvider querySupportService = new QueryParameterProvider(CacheUtils.getCache());
+ Object[] params = new Object[] {querySupportService.createQueryParameter()};
+ SelectResults result = (SelectResults) query.execute(params);
+ assertEquals("The query returned a wrong result set = " + result.asList(), true,
+ result.containsAll(new ArrayList<>(Arrays.asList(21L, 22L, 92L, 12L, 11L, 91L))));
+ }
+
+
+ public class PortfolioForParameterizedQueries implements Serializable {
+ long ID;
+ Collection<PortfolioData> dataList = new ArrayList<>();
+
+ public long getID() {
+ return ID;
+ }
+
+ public void setID(long ID) {
+ this.ID = ID;
+ }
+
+ public Collection<PortfolioData> getDataList() {
+ return dataList;
+ }
+
+ public void setDataList(
+ Collection<PortfolioData> dataList) {
+ this.dataList = dataList;
+ }
+
+ public PortfolioForParameterizedQueries(long ID) {
+ this.ID = ID;
+ }
+
+ @Override
+ public String toString() {
+ return "PortfolioParameter{" + "id=" + ID + ", data=" + dataList + "}";
+ }
+ }
+
+ public class PortfolioData {
+ List<Bound> bounds = new ArrayList<>();
+
+ public List<Bound> getBound() {
+ return bounds;
+ }
+
+ public void setLimits(List<Bound> bounds) {
+ this.bounds = bounds;
+ }
+
+ public PortfolioData(List<Bound> bounds) {
+ this.bounds = bounds;
+ }
+
+ @Override
+ public String toString() {
+ return "PortfolioData{bounds=" +
+ bounds +
+ '}';
+ }
+ }
+
+ public class Bound implements Serializable {
+ long ID;
+
+ public Bound(long ID) {
+ this.ID = ID;
+ }
+
+ public long getID() {
+ return ID;
+ }
+
+ public void setID(long ID) {
+ this.ID = ID;
+ }
+
+ @Override
+ public String toString() {
+ return "Bound{ID=" + ID + "}";
+ }
+ }
+
+ public class QueryParameterProvider {
+ private Cache cache;
+
+ public QueryParameterProvider(Cache cache) {
+ this.cache = cache;
+ }
+
+ public QueryParameter createQueryParameter() {
+ return new QueryParameter(
+ "<TRACE>select distinct ib from /intBound ib where ib.originalId in $1");
+ }
+
+ public class QueryParameter {
+ private final ExecutorService executorService = Executors.newSingleThreadExecutor();
+ private final Query query;
+
+ private QueryParameter(final String queryString) {
+ try {
+ this.query = executorService
+ .submit(() -> cache.getQueryService().newQuery(queryString)).get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Thread was unexpectedly interrupted", e);
+ } catch (ExecutionException e) {
+ throw new RuntimeException("Could not create query context", e);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private Iterable<Bound> getIntermediateBounds(final Set<Long> originalBoundIds) {
+ final Set<IntermediateBound> infos;
+ final List<Bound> intermediateBounds = new ArrayList<>();
+
+ try {
+ infos = executorService.submit(
+ () -> ((SelectResults<IntermediateBound>) query
+ .execute(new Object[] {originalBoundIds}))
+ .asSet())
+ .get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Thread was unexpectedly interrupted", e);
+ } catch (ExecutionException e) {
+ throw new RuntimeException("Could not query for temp limits", e);
+ }
+
+ if (infos != null) {
+ for (final IntermediateBound info : infos) {
+ if (info != null) {
+ final Bound intLimit = info.getBound();
+ if (intLimit != null) {
+ intermediateBounds.add(intLimit);
+ }
+ }
+ }
+ }
+ return intermediateBounds;
+ }
+
+ public Collection<Bound> getConcatenatedBoundsAndIntermediateBounds(
+ final PortfolioData portfolioData) {
+ final Map<Long, Bound> boundMap = getBounds(portfolioData);
+ return Lists.newArrayList(
+ Iterables.concat(boundMap.values(), getIntermediateBounds(boundMap.keySet())));
+ }
+
+ private Map<Long, Bound> getBounds(final PortfolioData portfolioData) {
+ final Map<Long, Bound> boundsMap = new HashMap<Long, Bound>();
+ if (portfolioData != null) {
+ for (final Bound bound : portfolioData.bounds) {
+ if (bound != null) {
+ boundsMap.put(bound.ID, bound);
+ }
+ }
+ }
+ return boundsMap;
+ }
+ }
+ }
+
+ public class IntermediateBound implements Serializable {
+ long ID;
+ long originalId;
+ Bound bound;
+
+ public long getOriginalId() {
+ return originalId;
+ }
+
+ public long getID() {
+ return ID;
+ }
+
+ public void setID(long ID) {
+ this.ID = ID;
+ }
+
+ public Bound getBound() {
+ return bound;
+ }
+
+ public IntermediateBound setOriginalId(long originalId) {
+ this.originalId = originalId;
+ return this;
+ }
+
+ public IntermediateBound setBound(Bound bound) {
+ this.bound = bound;
+ return this;
+ }
+
+ public IntermediateBound(long ID, long originalId, Bound bound) {
+ this.ID = ID;
+ this.originalId = originalId;
+ this.bound = bound;
+ }
+
+ @Override
+ public String toString() {
+ return "Bound{" +
+ "id=" + ID +
+ '}';
+ }
+
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledID.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledID.java
index c7e0ff0..d141014 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledID.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledID.java
@@ -61,7 +61,10 @@ public class CompiledID extends AbstractCompiledValue {
return _id;
}
-
+ @Override
+ public boolean hasIdentifierAtLeafNode() {
+ return true;
+ }
public int getType() {
return Identifier;
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java
index 413d534..d48a4bf 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIndexOperation.java
@@ -162,6 +162,15 @@ public class CompiledIndexOperation extends AbstractCompiledValue implements Map
return receiver;
}
+ @Override
+ public boolean hasIdentifierAtLeafNode() {
+ if (this.receiver.getType() == Identifier) {
+ return true;
+ } else {
+ return this.receiver.hasIdentifierAtLeafNode();
+ }
+ }
+
public CompiledValue getExpression() {
return indexExpr;
}
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledOperation.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledOperation.java
index 728b043..81ad28d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledOperation.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledOperation.java
@@ -91,6 +91,15 @@ public class CompiledOperation extends AbstractCompiledValue {
}
@Override
+ public boolean hasIdentifierAtLeafNode() {
+ if (this.receiver.getType() == Identifier) {
+ return true;
+ } else {
+ return this.receiver.hasIdentifierAtLeafNode();
+ }
+ }
+
+ @Override
public CompiledValue getReceiver() {
return this.getReceiver(null);
}
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledPath.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledPath.java
index f88c4bc..49e7963 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledPath.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledPath.java
@@ -164,6 +164,15 @@ public class CompiledPath extends AbstractCompiledValue {
}
@Override
+ public boolean hasIdentifierAtLeafNode() {
+ if (this._receiver.getType() == Identifier) {
+ return true;
+ } else {
+ return this._receiver.hasIdentifierAtLeafNode();
+ }
+ }
+
+ @Override
public void generateCanonicalizedExpression(StringBuilder clauseBuffer, ExecutionContext context)
throws AmbiguousNameException, TypeMismatchException, NameResolutionException {
// Asif: Canonicalize the tail ID. If the tail ID contains
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledValue.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledValue.java
index 3f34a88..13ff85c 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledValue.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledValue.java
@@ -141,4 +141,8 @@ public interface CompiledValue {
}
CompiledValue getReceiver();
+
+ default boolean hasIdentifierAtLeafNode() {
+ return false;
+ }
}
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/QueryUtils.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/QueryUtils.java
index 6d119db..7d04168 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/QueryUtils.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/QueryUtils.java
@@ -722,13 +722,17 @@ public class QueryUtils {
} else if (currentLevel.getCmpIteratorDefn().getCollectionExpr()
.getType() == OQLLexerTokenTypes.LITERAL_select) {
useDerivedResults = false;
- } else {
- key = getCompiledIdFromPath(currentLevel.getCmpIteratorDefn().getCollectionExpr()).getId()
- + ':' + currentLevel.getDefinition();
}
SelectResults c;
- if (useDerivedResults && derivedResults != null && derivedResults.containsKey(key)) {
- c = derivedResults.get(key);
+ CompiledValue path = currentLevel.getCmpIteratorDefn().getCollectionExpr();
+ if (useDerivedResults && derivedResults != null && path.hasIdentifierAtLeafNode()) {
+ key = getCompiledIdFromPath(path).getId()
+ + ':' + currentLevel.getDefinition();
+ if (derivedResults.containsKey(key)) {
+ c = derivedResults.get(key);
+ } else {
+ c = currentLevel.evaluateCollection(context);
+ }
} else {
c = currentLevel.evaluateCollection(context);
}
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/internal/AbstractCompiledValueTestJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/internal/AbstractCompiledValueTestJUnitTest.java
new file mode 100644
index 0000000..e43e94b
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/internal/AbstractCompiledValueTestJUnitTest.java
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.query.internal;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.geode.cache.query.internal.types.CollectionTypeImpl;
+
+@RunWith(JUnitParamsRunner.class)
+public class AbstractCompiledValueTestJUnitTest {
+
+ private CompiledValue[] getCompiledValuesWhichDoNotImplementGetReceiver() {
+ CompiledValue compiledValue1 = new CompiledID("testString");
+ CompiledValue compiledValue2 = new CompiledID("testString");
+ return new CompiledValue[] {
+ new CompiledAddition(compiledValue1, compiledValue2, 13),
+ new CompiledAggregateFunction(compiledValue1, 13),
+ new CompiledAddition(compiledValue1, compiledValue2, 13),
+ new CompiledBindArgument(1),
+ new CompiledComparison(compiledValue1, compiledValue2, 13),
+ new CompiledConstruction(Object.class, new ArrayList()),
+ new CompiledDivision(compiledValue1, compiledValue2, 13),
+ new CompiledFunction(new CompiledValue[] {compiledValue1, compiledValue2}, 13),
+ new CompiledGroupBySelect(true, true, compiledValue1, new ArrayList(), new ArrayList(),
+ new ArrayList<>(), compiledValue2, new ArrayList<>(), new ArrayList<>(),
+ new LinkedHashMap<>()),
+ new CompiledIn(compiledValue1, compiledValue2),
+ new CompiledIteratorDef("test", new CollectionTypeImpl(), compiledValue1),
+ new CompiledJunction(new CompiledValue[] {compiledValue1, compiledValue2}, 89),
+ new CompiledLike(compiledValue1, compiledValue2),
+ new CompiledLiteral(compiledValue1),
+ new CompiledMod(compiledValue1, compiledValue2, 13),
+ new CompiledMultiplication(compiledValue1, compiledValue2, 13),
+ new CompiledNegation(compiledValue1),
+ new CompiledRegion("test"),
+ new CompiledSortCriterion(true, compiledValue1),
+ new CompiledSubtraction(compiledValue1, compiledValue2, 13),
+ new CompiledUnaryMinus(compiledValue1),
+ new CompiledUndefined(compiledValue1, true),
+ new CompositeGroupJunction(13, compiledValue1),
+ new GroupJunction(13, new RuntimeIterator[] {}, true,
+ new CompiledValue[] {compiledValue1, compiledValue2})
+ };
+ }
+
+ private CompiledValue[] getCompiledValuesWhichImplementGetReceiver() {
+ CompiledValue compiledValue1 = new CompiledID("testString");
+ CompiledValue compiledValue2 = new CompiledID("testString");
+ return new CompiledValue[] {
+ new CompiledID("testString"),
+ new CompiledIndexOperation(compiledValue1, compiledValue2),
+ new CompiledOperation(compiledValue1, "test", new ArrayList()),
+ new CompiledPath(compiledValue1, "test")
+ };
+ }
+
+ @Test
+ @Parameters(method = "getCompiledValuesWhichDoNotImplementGetReceiver")
+ public void whenGetReceiverIsNotImplementedThenHasIdentifierAtLeafMustReturnFalse(
+ CompiledValue compiledValue) {
+ assertFalse(compiledValue.hasIdentifierAtLeafNode());
+ }
+
+ @Test
+ @Parameters(method = "getCompiledValuesWhichImplementGetReceiver")
+ public void whenGetReceiverIsImplementedThenHasIdentifierAtLeafMustReturnTrue(
+ CompiledValue compiledValue) {
+ assertTrue(compiledValue.hasIdentifierAtLeafNode());
+ }
+
+ @Test
+ public void whenLeafIsIdentifierAtTheLeafThenHasIdentifierAtLeafMustReturnTrue() {
+ CompiledValue compiledValue1 = new CompiledID("testString");
+ CompiledValue compiledValue2 = new CompiledID("testString");
+ CompiledIndexOperation compiledIndexOperation =
+ new CompiledIndexOperation(compiledValue1, compiledValue2);
+ CompiledPath compiledPath = new CompiledPath(compiledIndexOperation, "test");
+ assertTrue(compiledPath.hasIdentifierAtLeafNode());
+ }
+
+ @Test
+ public void whenLeafIsNotIndentifierThenHasIdentifierAtLeafMustReturnFalse() {
+ CompiledValue compiledValue1 = new CompiledBindArgument(1);
+ CompiledValue compiledValue2 = new CompiledBindArgument(1);
+ CompiledIndexOperation compiledIndexOperation =
+ new CompiledIndexOperation(compiledValue1, compiledValue2);
+ CompiledPath compiledPath = new CompiledPath(compiledIndexOperation, "test");
+ assertFalse(compiledPath.hasIdentifierAtLeafNode());
+ }
+
+}