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>

Reply via email to