[ASTERIXDB-2346][COMP] Constant folding should not fail on runtime exceptions
- user model changes: no - storage format changes: no - interface changes: no Details: - Constant folding rule should not fail on runtime exceptions - throw RuntimeDataException instead of java.lang.ArithmeticException from numeric operators Change-Id: I286551a98f57df798ce982228a66d6a1e3fc7304 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2542 Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Till Westmann <ti...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/cc9257eb Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/cc9257eb Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/cc9257eb Branch: refs/heads/release-0.9.4-pre-rc Commit: cc9257ebffe118bd85d76ae915ad03fe62503eb9 Parents: 8e4b57b Author: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Authored: Wed Mar 28 14:52:57 2018 -0700 Committer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Committed: Thu Mar 29 22:22:41 2018 -0700 ---------------------------------------------------------------------- .../optimizer/rules/ConstantFoldingRule.java | 71 +++++++++++--------- .../numeric/ifinf/ifinf.1.query.sqlpp | 3 +- .../runtimets/results/numeric/ifinf/ifinf.1.adm | 3 +- .../asterix/common/exceptions/ErrorCode.java | 1 + .../main/resources/asx_errormsg/en.properties | 1 + .../functions/NumericCaretDescriptor.java | 3 +- .../functions/NumericDivideDescriptor.java | 7 +- 7 files changed, 52 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java index fd66821..7e9328b 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java @@ -67,6 +67,7 @@ import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.algebricks.core.algebra.operators.logical.IOperatorSchema; import org.apache.hyracks.algebricks.core.algebra.visitors.ILogicalExpressionReferenceTransform; import org.apache.hyracks.algebricks.core.algebra.visitors.ILogicalExpressionVisitor; +import org.apache.hyracks.algebricks.core.config.AlgebricksConfig; import org.apache.hyracks.algebricks.core.jobgen.impl.JobGenContext; import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule; import org.apache.hyracks.algebricks.runtime.base.IScalarEvaluator; @@ -93,11 +94,13 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { BuiltinFunctions.FIELD_ACCESS_BY_INDEX, BuiltinFunctions.CAST_TYPE, BuiltinFunctions.META, BuiltinFunctions.META_KEY, BuiltinFunctions.RECORD_CONCAT, BuiltinFunctions.RECORD_CONCAT_STRICT); - /** Throws exceptions in substituiteProducedVariable, setVarType, and one getVarType method. */ + /** + * Throws exceptions in substituiteProducedVariable, setVarType, and one getVarType method. + */ private static final IVariableTypeEnvironment _emptyTypeEnv = new IVariableTypeEnvironment() { @Override - public boolean substituteProducedVariable(LogicalVariable v1, LogicalVariable v2) throws AlgebricksException { + public boolean substituteProducedVariable(LogicalVariable v1, LogicalVariable v2) { throw new IllegalStateException(); } @@ -108,12 +111,12 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { @Override public Object getVarType(LogicalVariable var, List<LogicalVariable> nonNullVariables, - List<List<LogicalVariable>> correlatedNullableVariableLists) throws AlgebricksException { + List<List<LogicalVariable>> correlatedNullableVariableLists) { throw new IllegalStateException(); } @Override - public Object getVarType(LogicalVariable var) throws AlgebricksException { + public Object getVarType(LogicalVariable var) { throw new IllegalStateException(); } @@ -170,14 +173,13 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { } @Override - public Pair<Boolean, ILogicalExpression> visitConstantExpression(ConstantExpression expr, Void arg) - throws AlgebricksException { + public Pair<Boolean, ILogicalExpression> visitConstantExpression(ConstantExpression expr, Void arg) { return new Pair<>(false, expr); } @Override public Pair<Boolean, ILogicalExpression> visitVariableReferenceExpression(VariableReferenceExpression expr, - Void arg) throws AlgebricksException { + Void arg) { return new Pair<>(false, expr); } @@ -194,31 +196,33 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { return new Pair<>(false, null); } - //Current List SerDe assumes a strongly typed list, so we do not constant fold the list constructors if they are not strongly typed - if (expr.getFunctionIdentifier().equals(BuiltinFunctions.UNORDERED_LIST_CONSTRUCTOR) - || expr.getFunctionIdentifier().equals(BuiltinFunctions.ORDERED_LIST_CONSTRUCTOR)) { - AbstractCollectionType listType = (AbstractCollectionType) TypeCastUtils.getRequiredType(expr); - if (listType != null && (listType.getItemType().getTypeTag() == ATypeTag.ANY - || listType.getItemType() instanceof AbstractCollectionType)) { - //case1: listType == null, could be a nested list inside a list<ANY> - //case2: itemType = ANY - //case3: itemType = a nested list - return new Pair<>(false, null); + try { + // Current List SerDe assumes a strongly typed list, so we do not constant fold the list constructors + // if they are not strongly typed + if (expr.getFunctionIdentifier().equals(BuiltinFunctions.UNORDERED_LIST_CONSTRUCTOR) + || expr.getFunctionIdentifier().equals(BuiltinFunctions.ORDERED_LIST_CONSTRUCTOR)) { + AbstractCollectionType listType = (AbstractCollectionType) TypeCastUtils.getRequiredType(expr); + if (listType != null && (listType.getItemType().getTypeTag() == ATypeTag.ANY + || listType.getItemType() instanceof AbstractCollectionType)) { + //case1: listType == null, could be a nested list inside a list<ANY> + //case2: itemType = ANY + //case3: itemType = a nested list + return new Pair<>(false, null); + } } - } - if (expr.getFunctionIdentifier().equals(BuiltinFunctions.FIELD_ACCESS_BY_NAME)) { - ARecordType rt = (ARecordType) _emptyTypeEnv.getType(expr.getArguments().get(0).getValue()); - String str = ConstantExpressionUtil.getStringConstant(expr.getArguments().get(1).getValue()); - int k = rt.getFieldIndex(str); - if (k >= 0) { - // wait for the ByNameToByIndex rule to apply - return new Pair<>(changed, expr); + if (expr.getFunctionIdentifier().equals(BuiltinFunctions.FIELD_ACCESS_BY_NAME)) { + ARecordType rt = (ARecordType) _emptyTypeEnv.getType(expr.getArguments().get(0).getValue()); + String str = ConstantExpressionUtil.getStringConstant(expr.getArguments().get(1).getValue()); + int k = rt.getFieldIndex(str); + if (k >= 0) { + // wait for the ByNameToByIndex rule to apply + return new Pair<>(changed, expr); + } } - } - IScalarEvaluatorFactory fact = jobGenCtx.getExpressionRuntimeProvider().createEvaluatorFactory(expr, - _emptyTypeEnv, _emptySchemas, jobGenCtx); - try { + IScalarEvaluatorFactory fact = jobGenCtx.getExpressionRuntimeProvider().createEvaluatorFactory(expr, + _emptyTypeEnv, _emptySchemas, jobGenCtx); + IScalarEvaluator eval = fact.createScalarEvaluator(null); eval.evaluate(null, p); Object t = _emptyTypeEnv.getType(expr); @@ -229,8 +233,11 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { bbis.setByteBuffer(ByteBuffer.wrap(p.getByteArray(), p.getStartOffset(), p.getLength()), 0); IAObject o = (IAObject) serde.deserialize(dis); return new Pair<>(true, new ConstantExpression(new AsterixConstantValue(o))); - } catch (HyracksDataException e) { - throw new AlgebricksException(e); + } catch (HyracksDataException | AlgebricksException e) { + if (AlgebricksConfig.ALGEBRICKS_LOGGER.isDebugEnabled()) { + AlgebricksConfig.ALGEBRICKS_LOGGER.debug("Exception caught at constant folding: " + e, e); + } + return new Pair<>(false, null); } } @@ -267,7 +274,7 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule { return changed; } - private boolean checkArgs(AbstractFunctionCallExpression expr) throws AlgebricksException { + private boolean checkArgs(AbstractFunctionCallExpression expr) { for (Mutable<ILogicalExpression> r : expr.getArguments()) { if (r.getValue().getExpressionTag() != LogicalExpressionTag.CONSTANT) { return false; http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/numeric/ifinf/ifinf.1.query.sqlpp ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/numeric/ifinf/ifinf.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/numeric/ifinf/ifinf.1.query.sqlpp index 3b12ead..3b395bc 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/numeric/ifinf/ifinf.1.query.sqlpp +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/numeric/ifinf/ifinf.1.query.sqlpp @@ -39,6 +39,7 @@ from [ [ 17, ifinf(float("INF"), double("-INF"), 2) ], [ 18, isnull(ifinf(double("INF"), double("-INF"), [], 2)) ], [ 19, ismissing(if_inf(double("INF"), double("-INF"), missing, 2)) ], - [ 20, tostring(ifinf(float("INF"), float("NaN"), 2)) ] + [ 20, tostring(ifinf(float("INF"), float("NaN"), 2)) ], + [ 21, if_inf(2, 1/0) ] ] t order by t[0] \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-app/src/test/resources/runtimets/results/numeric/ifinf/ifinf.1.adm ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/numeric/ifinf/ifinf.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/numeric/ifinf/ifinf.1.adm index c45efa6..9f56be4 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/results/numeric/ifinf/ifinf.1.adm +++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/numeric/ifinf/ifinf.1.adm @@ -18,4 +18,5 @@ [ 17, 2 ] [ 18, true ] [ 19, true ] -[ 20, "NaN" ] \ No newline at end of file +[ 20, "NaN" ] +[ 21, 2 ] \ No newline at end of file http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java ---------------------------------------------------------------------- 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 8146797..b9a5b29 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 @@ -74,6 +74,7 @@ public class ErrorCode { public static final int INVALID_TYPE_CASTING_MATH_FUNCTION = 31; public static final int REJECT_BAD_CLUSTER_STATE = 32; public static final int REJECT_NODE_UNREGISTERED = 33; + public static final int DIVISION_BY_ZERO = 34; public static final int INSTANTIATION_ERROR = 100; http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties ---------------------------------------------------------------------- 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 f9188b8..92aac98 100644 --- a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties +++ b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties @@ -67,6 +67,7 @@ 31 = Invalid type-casting math function: %1$s for converting %2$s to %3$s 32 = Cannot execute request, cluster is %1$s 33 = Node is not registered with the CC +34 = Division by Zero. 100 = Unable to instantiate class %1$s http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java index 3b72072..0079a8a 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java @@ -22,6 +22,7 @@ import org.apache.asterix.om.functions.BuiltinFunctions; import org.apache.asterix.om.functions.IFunctionDescriptor; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; import org.apache.asterix.om.types.ATypeTag; +import org.apache.asterix.runtime.exceptions.OverflowException; import org.apache.asterix.runtime.exceptions.UnsupportedTypeException; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.api.exceptions.HyracksDataException; @@ -44,7 +45,7 @@ public class NumericCaretDescriptor extends AbstractNumericArithmeticEval { @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { if (rhs > Integer.MAX_VALUE) { - throw new ArithmeticException("Exponent cannot be larger than 2^31-1"); + throw new OverflowException(getIdentifier()); } return LongMath.checkedPow(lhs, (int) rhs); } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cc9257eb/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java index 7cda149..77c94bf 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java @@ -18,10 +18,13 @@ */ package org.apache.asterix.runtime.evaluators.functions; +import org.apache.asterix.common.exceptions.ErrorCode; +import org.apache.asterix.common.exceptions.RuntimeDataException; import org.apache.asterix.om.functions.BuiltinFunctions; import org.apache.asterix.om.functions.IFunctionDescriptor; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; import org.apache.asterix.om.types.ATypeTag; +import org.apache.asterix.runtime.exceptions.OverflowException; import org.apache.asterix.runtime.exceptions.UnsupportedTypeException; import org.apache.hyracks.algebricks.common.exceptions.NotImplementedException; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; @@ -44,10 +47,10 @@ public class NumericDivideDescriptor extends AbstractNumericArithmeticEval { @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { if (rhs == 0) { - throw new ArithmeticException("Division by Zero."); + throw new RuntimeDataException(ErrorCode.DIVISION_BY_ZERO); } if ((lhs == Long.MIN_VALUE) && (rhs == -1L)) { - throw new ArithmeticException(("Overflow in integer division")); + throw new OverflowException(getIdentifier()); } return lhs / rhs; }
