You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ja...@apache.org on 2016/12/14 19:31:33 UTC
geode git commit: GEODE-2183: Region.query now allows full syntax as
a predicate on server
Repository: geode
Updated Branches:
refs/heads/develop a225060f4 -> d65c1fe26
GEODE-2183: Region.query now allows full syntax as a predicate on server
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/d65c1fe2
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/d65c1fe2
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/d65c1fe2
Branch: refs/heads/develop
Commit: d65c1fe26289d6d0d34acfc83c9a0c92f6c0fb3a
Parents: a225060
Author: Jason Huynh <hu...@gmail.com>
Authored: Thu Dec 1 13:47:15 2016 -0800
Committer: Jason Huynh <hu...@gmail.com>
Committed: Wed Dec 14 11:29:13 2016 -0800
----------------------------------------------------------------------
.../java/org/apache/geode/cache/Region.java | 2 -
.../geode/internal/cache/LocalRegion.java | 51 +++++++++++++-------
.../geode/cache/query/RegionJUnitTest.java | 29 +++++++++++
.../cache/query/dunit/RemoteQueryDUnitTest.java | 2 +-
.../partitioned/PRInvalidQueryJUnitTest.java | 51 --------------------
5 files changed, 63 insertions(+), 72 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/d65c1fe2/geode-core/src/main/java/org/apache/geode/cache/Region.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/Region.java b/geode-core/src/main/java/org/apache/geode/cache/Region.java
index be70157..3eef543 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/Region.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/Region.java
@@ -1240,8 +1240,6 @@ public interface Region<K, V> extends ConcurrentMap<K, V> {
* When executed from a client, this method always runs on the server. However application should
* use QueryService to execute queries.
*
- * When executed from a client, this method always runs on the server. However application should
- * use QueryService to execute queries.
*
* @see Pool#getQueryService
* @see Cache#getQueryService()
http://git-wip-us.apache.org/repos/asf/geode/blob/d65c1fe2/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 4349359..1343395 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -88,6 +88,7 @@ import org.apache.geode.cache.query.IndexType;
import org.apache.geode.cache.query.MultiIndexCreationException;
import org.apache.geode.cache.query.NameResolutionException;
import org.apache.geode.cache.query.QueryException;
+import org.apache.geode.cache.query.QueryInvalidException;
import org.apache.geode.cache.query.QueryInvocationTargetException;
import org.apache.geode.cache.query.QueryService;
import org.apache.geode.cache.query.SelectResults;
@@ -10897,22 +10898,8 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
String queryString = null;
// Trim whitespace
- predicate = predicate.trim();
-
- // Compare the query patterns to the 'predicate'. If one matches,
- // send it as is to the server
- boolean matches = false;
- for (int i = 0; i < QUERY_PATTERNS.length; i++) {
- if (QUERY_PATTERNS[i].matcher(predicate).matches()) {
- matches = true;
- break;
- }
- }
- if (matches) {
- queryString = predicate;
- } else {
- queryString = "select * from " + getFullPath() + " this where " + predicate;
- }
+ queryString = constructRegionQueryString(predicate.trim());
+
try {
results = getServerProxy().query(queryString, null);
} catch (Exception e) {
@@ -10923,14 +10910,42 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
throw new QueryInvocationTargetException(e.getMessage(), cause);
}
} else {
+ Object[] params = new Object[0];
QueryService qs = getGemFireCache().getLocalQueryService();
- String queryStr = "select * from " + getFullPath() + " this where " + predicate;
+ String queryStr = constructRegionQueryString(predicate.trim());
DefaultQuery query = (DefaultQuery) qs.newQuery(queryStr);
- results = (SelectResults) query.execute(new Object[0]);
+ if (query.getRegionsInQuery(params).size() != 1) {
+ throw new QueryInvalidException(
+ "Prevent multiple region query from being executed through region.query()");
+ }
+ results = (SelectResults) query.execute(params);
}
return results;
}
+ private String constructRegionQueryString(final String predicate) throws QueryInvalidException {
+ final String queryString;// Compare the query patterns to the 'predicate'. If one matches,
+ // send it as is to the server
+ boolean matches = false;
+ for (int i = 0; i < QUERY_PATTERNS.length; i++) {
+ if (QUERY_PATTERNS[i].matcher(predicate).matches()) {
+ if (!predicate.contains(getName())) {
+ throw new QueryInvalidException(
+ "Should not execute region.query with a different region in the from clause: "
+ + getName() + " was not present in:" + predicate);
+ }
+ matches = true;
+ break;
+ }
+ }
+ if (matches) {
+ queryString = predicate;
+ } else {
+ queryString = "select * from " + getFullPath() + " this where " + predicate;
+ }
+ return queryString;
+ }
+
/**
* Called from the RegionMap when the {@link RegionEntry} is synchronized and it is safe to make
* decisions about Entry state
http://git-wip-us.apache.org/repos/asf/geode/blob/d65c1fe2/geode-core/src/test/java/org/apache/geode/cache/query/RegionJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/RegionJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/RegionJUnitTest.java
index ad82077..2b3238e 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/RegionJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/RegionJUnitTest.java
@@ -20,8 +20,10 @@ import static org.junit.Assert.fail;
import java.util.Iterator;
+import org.apache.geode.cache.RegionShortcut;
import org.junit.After;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -35,6 +37,7 @@ import org.apache.geode.internal.cache.LocalRegion;
// for internal access test
import org.apache.geode.internal.cache.RegionEntry;
import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.junit.rules.ExpectedException;
/**
* RegionJUnitTest.java
@@ -54,6 +57,32 @@ public class RegionJUnitTest {
QueryService qs;
Cache cache;
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void regionQueryWithFullQueryShouldNotFail() throws Exception {
+ SelectResults results = region.query("SeLeCt * FROM /pos where ID = 1");
+ assertEquals(1, results.size());
+
+ results = region.query("SELECT * FROM /pos");
+ assertEquals(4 /* num entries added in setup */, results.size());
+ }
+
+ @Test
+ public void regionQueryExecuteWithFullQueryWithDifferentRegionShouldFail() throws Exception {
+ expectedException.expect(QueryInvalidException.class);
+ cache.createRegionFactory(RegionShortcut.REPLICATE).create("otherRegion");
+ SelectResults results = region.query("select * FROM /otherRegion where ID = 1");
+ }
+
+ @Test
+ public void regionQueryExecuteWithMulipleRegionsInFullQueryShouldFail() throws Exception {
+ expectedException.expect(QueryInvalidException.class);
+ cache.createRegionFactory(RegionShortcut.REPLICATE).create("otherRegion");
+ SelectResults results = region.query("select * FROM /pos, /otherRegion where ID = 1");
+ }
+
@Test
public void testShortcutMethods() throws Exception {
http://git-wip-us.apache.org/repos/asf/geode/blob/d65c1fe2/geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java
index 8088374..7d0b469 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java
@@ -1284,7 +1284,7 @@ public class RemoteQueryDUnitTest extends JUnit4CacheTestCase {
vm1.invoke(new CacheSerializableRunnable("Execute queries") {
public void run2() throws CacheException {
Region region = getRootRegion().getSubregion(name);
- String queryString = "select distinct * from /" + name + ",/" + name + "_2";
+ String queryString = "select distinct * from /" + name;
// SelectResults results = null;
try {
region.query(queryString);
http://git-wip-us.apache.org/repos/asf/geode/blob/d65c1fe2/geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRInvalidQueryJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRInvalidQueryJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRInvalidQueryJUnitTest.java
index 6239b54..b56ad08 100755
--- a/geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRInvalidQueryJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRInvalidQueryJUnitTest.java
@@ -48,57 +48,6 @@ public class PRInvalidQueryJUnitTest {
}
/**
- * Tests the execution of an Invalid query <br>
- * (of the nature Select Distinct * from /Portfolios ) <br>
- * on a PartitionedRegion created on a single data store. <br>
- * 1. Creates a PR with redundancy=0 on a single VM.<br>
- * 2. Puts some test Objects in cache.<br>
- * 3. Fires querie on the data and verifies the result.<br>
- * 4. Since region#query() doesn't support this type of query syntax it should throw
- * QueryInvalidException <br>
- *
- * @throws Exception
- */
- @Test
- public void testInvalidQueryOnSingleDS() throws Exception {
- logger.info("PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: Test Started ");
- Region region = PartitionedRegionTestHelper.createPartitionedRegion(regionName, "100", 0);
-
- logger
- .info("PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: creating portfolioData objects");
- PortfolioData[] portfolios = new PortfolioData[100];
- for (int j = 0; j < 100; j++) {
- portfolios[j] = new PortfolioData(j);
- }
- populateData(region, portfolios);
-
- logger.info("PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: creating Select Query");
-
- String queryString = "SELECT DISTINCT * FROM /Portfolios WHERE pkid < '5'";
-
- final String expectedQueryInvalidException = QueryInvalidException.class.getName();
- logger.info(
- "<ExpectedException action=add>" + expectedQueryInvalidException + "</ExpectedException>");
- try {
- region.query(queryString);
- fail(
- "PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: Expected an Invalid Query Exception for the query :"
- + queryString + " this is not supported for region#query()");
- } catch (QueryInvalidException qe) {
-
- logger.info(
- "PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: Caught an Invalid Query Exception for the query :"
- + queryString + " this is not supported for region#query()");
-
- }
-
- logger.info("PRInvalidQueryJUnitTest#testInvalidQueryOnSingleDS: Test Ended");
-
- logger.info("<ExpectedException action=remove>" + expectedQueryInvalidException
- + "</ExpectedException>");
- }
-
- /**
* Populates the region with the Objects stores in the data Object array.
*
* @param region