Michael Blow has submitted this change and it was merged. Change subject: Raise exceptions for invalid query parameters. ......................................................................
Raise exceptions for invalid query parameters. Change-Id: I7dfc7fb76d76a3af243b0db674ee730aa1748df3 Reviewed-on: https://asterix-gerrit.ics.uci.edu/1627 Reviewed-by: Michael Blow <mb...@apache.org> Tested-by: Michael Blow <mb...@apache.org> --- M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java M asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties 4 files changed, 26 insertions(+), 12 deletions(-) Approvals: Michael Blow: Looks good to me, approved; Verified diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java index 7896424..f0e8588 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java @@ -226,11 +226,14 @@ int frameSize = compilerProperties.getFrameSize(); Map<String, String> querySpecificConfig = metadataProvider.getConfig(); validateConfig(querySpecificConfig); // Validates the user-overridden query parameters. - int sortFrameLimit = getFrameLimit(querySpecificConfig.get(CompilerProperties.COMPILER_SORTMEMORY_KEY), + int sortFrameLimit = getFrameLimit(CompilerProperties.COMPILER_SORTMEMORY_KEY, + querySpecificConfig.get(CompilerProperties.COMPILER_SORTMEMORY_KEY), compilerProperties.getSortMemorySize(), frameSize, MIN_FRAME_LIMIT_FOR_SORT); - int groupFrameLimit = getFrameLimit(querySpecificConfig.get(CompilerProperties.COMPILER_GROUPMEMORY_KEY), + int groupFrameLimit = getFrameLimit(CompilerProperties.COMPILER_GROUPMEMORY_KEY, + querySpecificConfig.get(CompilerProperties.COMPILER_GROUPMEMORY_KEY), compilerProperties.getGroupMemorySize(), frameSize, MIN_FRAME_LIMIT_FOR_GROUP_BY); - int joinFrameLimit = getFrameLimit(querySpecificConfig.get(CompilerProperties.COMPILER_JOINMEMORY_KEY), + int joinFrameLimit = getFrameLimit(CompilerProperties.COMPILER_JOINMEMORY_KEY, + querySpecificConfig.get(CompilerProperties.COMPILER_JOINMEMORY_KEY), compilerProperties.getJoinMemorySize(), frameSize, MIN_FRAME_LIMIT_FOR_JOIN); OptimizationConfUtil.getPhysicalOptimizationConfig().setFrameSize(frameSize); OptimizationConfUtil.getPhysicalOptimizationConfig().setMaxFramesExternalSort(sortFrameLimit); @@ -386,7 +389,7 @@ // Chooses the location constraints, i.e., whether to use storage parallelism or use a user-sepcified number // of cores. - private AlgebricksAbsolutePartitionConstraint chooseLocations(IClusterInfoCollector clusterInfoCollector, + private static AlgebricksAbsolutePartitionConstraint chooseLocations(IClusterInfoCollector clusterInfoCollector, int parallelismHint, AlgebricksAbsolutePartitionConstraint storageLocations) throws AlgebricksException { try { Map<String, NodeControllerInfo> ncMap = clusterInfoCollector.getNodeControllerInfos(); @@ -408,7 +411,7 @@ // Computes the location constraints based on user-configured parallelism parameter. // Note that the parallelism parameter is only a hint -- it will not be respected if it is too small or too large. - private AlgebricksAbsolutePartitionConstraint getComputationLocations(Map<String, NodeControllerInfo> ncMap, + private static AlgebricksAbsolutePartitionConstraint getComputationLocations(Map<String, NodeControllerInfo> ncMap, int parallelismHint) { // Unifies the handling of non-positive parallelism. int parallelism = parallelismHint <= 0 ? -2 * ncMap.size() : parallelismHint; @@ -448,7 +451,7 @@ } // Gets the total number of available cores in the cluster. - private int getTotalNumCores(Map<String, NodeControllerInfo> ncMap) { + private static int getTotalNumCores(Map<String, NodeControllerInfo> ncMap) { int sum = 0; for (Map.Entry<String, NodeControllerInfo> entry : ncMap.entrySet()) { sum += entry.getValue().getNumAvailableCores(); @@ -457,24 +460,30 @@ } // Gets the frame limit. - private int getFrameLimit(String parameter, long memBudgetInConfiguration, int frameSize, int minFrameLimit) + private static int getFrameLimit(String parameterName, String parameter, long memBudgetInConfiguration, + int frameSize, + int minFrameLimit) throws AlgebricksException { IOptionType<Long> longBytePropertyInterpreter = OptionTypes.LONG_BYTE_UNIT; long memBudget = parameter == null ? memBudgetInConfiguration : longBytePropertyInterpreter.parse(parameter); int frameLimit = (int) (memBudget / frameSize); + if (frameLimit < minFrameLimit) { + throw AsterixException.create(ErrorCode.COMPILATION_BAD_QUERY_PARAMETER_VALUE, parameterName, + frameSize * minFrameLimit); + } // Sets the frame limit to the minimum frame limit if the caculated frame limit is too small. return Math.max(frameLimit, minFrameLimit); } // Gets the parallelism parameter. - private int getParallelism(String parameter, int parallelismInConfiguration) { + private static int getParallelism(String parameter, int parallelismInConfiguration) { IOptionType<Integer> integerIPropertyInterpreter = OptionTypes.INTEGER; return parameter == null ? parallelismInConfiguration : integerIPropertyInterpreter.parse(parameter); } // Validates if the query contains unsupported query parameters. - private void validateConfig(Map<String, String> config) throws AlgebricksException { + private static void validateConfig(Map<String, String> config) throws AlgebricksException { for (String parameterName : config.keySet()) { if (!CONFIGURABLE_PARAMETER_NAMES.contains(parameterName)) { throw AsterixException.create(ErrorCode.COMPILATION_UNSUPPORTED_QUERY_PARAMETER, parameterName); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 557e802..3c075e4 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -6877,7 +6877,8 @@ </test-case> <test-case FilePath="tpch-sql-sugar"> <compilation-unit name="q17_large_gby_variant_parameter"> - <output-dir compare="Text">q17_large_gby_variant</output-dir> + <output-dir compare="Text">none</output-dir> + <expected-error>Invalid query parameter compiler.groupmemory -- value has to be greater than or equal to</expected-error> </compilation-unit> </test-case> <test-case FilePath="tpch-sql-sugar"> @@ -6907,7 +6908,8 @@ </test-case> <test-case FilePath="tpch-sql-sugar"> <compilation-unit name="q01_pricing_summary_report_parameter"> - <output-dir compare="Text">q01_pricing_summary_report_nt</output-dir> + <output-dir compare="Text">none</output-dir> + <expected-error>Invalid query parameter compiler.sortmemory -- value has to be greater than or equal to</expected-error> </compilation-unit> </test-case> <test-case FilePath="tpch-sql-sugar"> @@ -6967,7 +6969,8 @@ </test-case> <test-case FilePath="tpch-sql-sugar"> <compilation-unit name="q09_product_type_profit_parameter"> - <output-dir compare="Text">q09_product_type_profit_nt</output-dir> + <output-dir compare="Text">none</output-dir> + <expected-error>Invalid query parameter compiler.joinmemory -- value has to be greater than or equal to</expected-error> </compilation-unit> </test-case> <test-case FilePath="tpch-sql-sugar"> diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java index 0c133ae..f521801 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java @@ -94,6 +94,7 @@ public static final int NO_TOKENIZER_FOR_TYPE = 1034; public static final int INCOMPATIBLE_SEARCH_MODIFIER = 1035; public static final int UNKNOWN_SEARCH_MODIFIER = 1036; + public static final int COMPILATION_BAD_QUERY_PARAMETER_VALUE = 1037; // Feed errors public static final int DATAFLOW_ILLEGAL_STATE = 3001; diff --git a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties index 6cf2a45..9e1ad76 100644 --- a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties +++ b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties @@ -80,6 +80,7 @@ 1034 = Tokenizer is not applicable to the given index kind %1$s 1035 = Incompatible search modifier %1$s for index type %2$s 1036 = Unknown search modifier type %1$s +1037 = Invalid query parameter %1$s -- value has to be greater than or equal to %2$s bytes # Feed Errors 3001 = Illegal state. -- To view, visit https://asterix-gerrit.ics.uci.edu/1627 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7dfc7fb76d76a3af243b0db674ee730aa1748df3 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org>