geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jasonhu...@apache.org
Subject geode git commit: GEODE-2183: Region.query now allows full syntax as a predicate on server
Date Wed, 14 Dec 2016 19:31:33 GMT
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 <huynhja@gmail.com>
Authored: Thu Dec 1 13:47:15 2016 -0800
Committer: Jason Huynh <huynhja@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


Mime
View raw message