Repository: incubator-geode Updated Branches: refs/heads/develop db4ad02f0 -> e0e2c7e69
GEODE-1907: QueryDataFunction now adds LIMIT clause if space is missing after FROM clause This closes #249 Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/e0e2c7e6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e0e2c7e6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e0e2c7e6 Branch: refs/heads/develop Commit: e0e2c7e69bf6b046b44e9bf7921a561c6b8c49ba Parents: db4ad02 Author: Jared Stewart <[email protected]> Authored: Thu Sep 29 12:47:15 2016 -0700 Committer: Kirk Lund <[email protected]> Committed: Fri Sep 30 13:24:38 2016 -0700 ---------------------------------------------------------------------- .../internal/beans/QueryDataFunction.java | 43 +++-------- .../QueryDataFunctionApplyLimitClauseTest.java | 78 ++++++++++++++++++++ 2 files changed, 90 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e0e2c7e6/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java index bcfbf43..de53aef 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/QueryDataFunction.java @@ -88,7 +88,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity private static final int LIMIT = 3; private static final int QUERY_RESULTSET_LIMIT = 4; private static final int QUERY_COLLECTIONS_DEPTH = 5; - private static final String SELECT_EXPR = "\\s*SELECT\\s+.+\\s+FROM\\s+.+"; + private static final String SELECT_EXPR = "\\s*SELECT\\s+.+\\s+FROM.+"; private static final Pattern SELECT_EXPR_PATTERN = Pattern.compile(SELECT_EXPR, Pattern.CASE_INSENSITIVE); private static final String SELECT_WITH_LIMIT_EXPR = "\\s*SELECT\\s+.+\\s+FROM(\\s+|(.*\\s+))LIMIT\\s+[0-9]+.*"; private static final Pattern SELECT_WITH_LIMIT_EXPR_PATTERN = Pattern.compile(SELECT_WITH_LIMIT_EXPR, Pattern.CASE_INSENSITIVE); @@ -122,13 +122,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity return ManagementConstants.QUERY_DATA_FUNCTION; } - private QueryDataFunctionResult selectWithType(final FunctionContext context, - String queryString, - final boolean showMember, - final String regionName, - final int limit, - final int queryResultSetLimit, - final int queryCollectionsDepth) throws Exception { + private QueryDataFunctionResult selectWithType(final FunctionContext context, String queryString, final boolean showMember, final String regionName, final int limit, final int queryResultSetLimit, final int queryCollectionsDepth) throws Exception { Cache cache = CacheFactory.getAnyInstance(); Function loclQueryFunc = new LocalQueryFunction("LocalQueryFunction", regionName, showMember).setOptimizeForWrite(true); queryString = applyLimitClause(queryString, limit, queryResultSetLimit); @@ -216,19 +210,19 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity * * @return a string having limit clause */ - private static String applyLimitClause(final String query, int limit, final int queryResultSetLimit) { + protected static String applyLimitClause(final String query, int limit, final int queryResultSetLimit) { Matcher matcher = SELECT_EXPR_PATTERN.matcher(query); if (matcher.matches()) { Matcher limit_matcher = SELECT_WITH_LIMIT_EXPR_PATTERN.matcher(query); - boolean matchResult = limit_matcher.matches(); + boolean queryAlreadyHasLimitClause = limit_matcher.matches(); - if (!matchResult) { + if (!queryAlreadyHasLimitClause) { if (limit == 0) { limit = queryResultSetLimit; } - String result = new String(query); + String result = query; result += " LIMIT " + limit; return result; } @@ -241,9 +235,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity try { if (members.size() == 1) { DistributedMember member = members.iterator().next(); - ResultCollector collector = FunctionService.onMember(member) - .withArgs(functionArgs) - .execute(ManagementConstants.QUERY_DATA_FUNCTION); + ResultCollector collector = FunctionService.onMember(member).withArgs(functionArgs).execute(ManagementConstants.QUERY_DATA_FUNCTION); List list = (List) collector.getResult(); Object object = null; if (list.size() > 0) { @@ -272,9 +264,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity } } else { // More than 1 Member - ResultCollector coll = FunctionService.onMembers(members) - .withArgs(functionArgs) - .execute(ManagementConstants.QUERY_DATA_FUNCTION); + ResultCollector coll = FunctionService.onMembers(members).withArgs(functionArgs).execute(ManagementConstants.QUERY_DATA_FUNCTION); List list = (List) coll.getResult(); Object object = list.get(0); @@ -324,12 +314,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity } } - public static Object queryData(final String query, - final String members, - final int limit, - final boolean zipResult, - final int queryResultSetLimit, - final int queryCollectionsDepth) throws Exception { + public static Object queryData(final String query, final String members, final int limit, final boolean zipResult, final int queryResultSetLimit, final int queryCollectionsDepth) throws Exception { if (query == null || query.isEmpty()) { return new JsonisedErroMessage(ManagementStrings.QUERY__MSG__QUERY_EMPTY.toLocalizedString()).toString(); @@ -366,8 +351,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity if (inputMembers != null && inputMembers.size() > 0) { if (!associatedMembers.containsAll(inputMembers)) { - return new JsonisedErroMessage(ManagementStrings.QUERY__MSG__REGIONS_NOT_FOUND_ON_MEMBERS.toLocalizedString(regionPath)) - .toString(); + return new JsonisedErroMessage(ManagementStrings.QUERY__MSG__REGIONS_NOT_FOUND_ON_MEMBERS.toLocalizedString(regionPath)).toString(); } } } @@ -382,8 +366,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity for (String regionPath : regionsInQuery) { DistributedRegionMXBean regionMBean = service.getDistributedRegionMXBean(regionPath); - if (regionMBean.getRegionType().equals(DataPolicy.PARTITION.toString()) || - regionMBean.getRegionType().equals(DataPolicy.PERSISTENT_PARTITION.toString())) { + if (regionMBean.getRegionType().equals(DataPolicy.PARTITION.toString()) || regionMBean.getRegionType().equals(DataPolicy.PERSISTENT_PARTITION.toString())) { return new JsonisedErroMessage(ManagementStrings.QUERY__MSG__JOIN_OP_EX.toLocalizedString()).toString(); } } @@ -444,7 +427,7 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity public String toString() { return gFJsonObject.toString(); } - + } /** @@ -455,8 +438,6 @@ public class QueryDataFunction extends FunctionAdapter implements InternalEntity * @param query input query * * @return a set of regions involved in the query - * - * @throws QueryInvalidException */ private static Set<String> compileQuery(final Cache cache, final String query) throws QueryInvalidException { QCompiler compiler = new QCompiler(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e0e2c7e6/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java new file mode 100644 index 0000000..7270a2b --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java @@ -0,0 +1,78 @@ +/* + * 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.management.internal.beans; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.test.junit.categories.UnitTest; + + +@Category(UnitTest.class) +public class QueryDataFunctionApplyLimitClauseTest { + + private String selectQuery; + private int limit_0; + private int limit_10; + private int queryResultSetLimit_100; + + @Before + public void setUp() throws Exception { + this.selectQuery = "SELECT * FROM /MyRegion"; + this.limit_0 = 0; + this.limit_10 = 10; + this.queryResultSetLimit_100 = 100; + } + + @Test + public void applyLimitClauseDoesNothingIfLimitClauseSpecified() { + String limitClause = " LIMIT 50"; + String selectQueryWithLimit = selectQuery + limitClause; + assertThat(QueryDataFunction.applyLimitClause(selectQueryWithLimit, limit_10, queryResultSetLimit_100)) + .isEqualTo(selectQueryWithLimit); + } + + @Test + public void applyLimitClauseAddsQueryResultSetLimit() { + assertThat(QueryDataFunction.applyLimitClause(selectQuery, limit_0, queryResultSetLimit_100)).isEqualTo(selectQuery + " LIMIT " + queryResultSetLimit_100); + } + + @Test + public void applyLimitClausePrefersLimitOverQueryResultSetLimit() { + assertThat(QueryDataFunction.applyLimitClause(selectQuery, limit_10, queryResultSetLimit_100)).isEqualTo(selectQuery + " LIMIT " + limit_10); + } + + @Test // GEODE-1907 + public void applyLimitClauseAddsQueryResultSetLimitIfMissingSpaceAfterFrom() { + String selectQueryMissingSpaceAfterFrom = "SELECT * FROM/MyRegion"; + assertThat(QueryDataFunction.applyLimitClause(selectQueryMissingSpaceAfterFrom, limit_0, queryResultSetLimit_100)) + .isEqualTo(selectQueryMissingSpaceAfterFrom + " LIMIT " + queryResultSetLimit_100); + } + + @Test + public void applyLimitClauseDoesNotAddQueryResultSetLimitIfMissingSpaceAfterFromButLimitIsPresent() { + String selectQueryMissingSpaceAfterFromWithLimit = "SELECT * FROM/MyRegion LIMIT " + limit_10; + assertThat(QueryDataFunction.applyLimitClause(selectQueryMissingSpaceAfterFromWithLimit, limit_0, queryResultSetLimit_100)) + .isEqualTo(selectQueryMissingSpaceAfterFromWithLimit); + } + +} \ No newline at end of file
