Return-Path: X-Original-To: apmail-asterixdb-commits-archive@minotaur.apache.org Delivered-To: apmail-asterixdb-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id CF84D18F78 for ; Tue, 10 Nov 2015 16:41:57 +0000 (UTC) Received: (qmail 20443 invoked by uid 500); 10 Nov 2015 16:41:57 -0000 Delivered-To: apmail-asterixdb-commits-archive@asterixdb.apache.org Received: (qmail 20401 invoked by uid 500); 10 Nov 2015 16:41:57 -0000 Mailing-List: contact commits-help@asterixdb.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@asterixdb.incubator.apache.org Delivered-To: mailing list commits@asterixdb.incubator.apache.org Received: (qmail 20391 invoked by uid 99); 10 Nov 2015 16:41:57 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Nov 2015 16:41:57 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 3B128C80EC for ; Tue, 10 Nov 2015 16:41:57 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.436 X-Spam-Level: * X-Spam-Status: No, score=1.436 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.345, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-eu-west.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id bfOnMQnGQrdU for ; Tue, 10 Nov 2015 16:41:48 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-eu-west.apache.org (ASF Mail Server at mx1-eu-west.apache.org) with SMTP id 187B820314 for ; Tue, 10 Nov 2015 16:41:46 +0000 (UTC) Received: (qmail 20331 invoked by uid 99); 10 Nov 2015 16:41:46 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Nov 2015 16:41:46 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 21173DFD86; Tue, 10 Nov 2015 16:41:46 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: imaxon@apache.org To: commits@asterixdb.incubator.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: incubator-asterixdb git commit: ASTERIXDB-1166: Fix numeric overflow guarding Date: Tue, 10 Nov 2015 16:41:46 +0000 (UTC) Repository: incubator-asterixdb Updated Branches: refs/heads/master b87772177 -> 89bbc2726 ASTERIXDB-1166: Fix numeric overflow guarding This should stop the issues with numerics acting funny. Instead of reinventing the wheel here and trying to detect when an overflow may happen, I opted to use the new methods in Java 8 that are intended to guard against overflow where it's not desired. Also, fix the docker image to use Java 8. Change-Id: I520b22861a76346caca646122bc736e66c8d8b1f Reviewed-on: https://asterix-gerrit.ics.uci.edu/478 Reviewed-by: Cameron Samak Tested-by: Jenkins Reviewed-by: Till Westmann Project: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/commit/89bbc272 Tree: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/tree/89bbc272 Diff: http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/diff/89bbc272 Branch: refs/heads/master Commit: 89bbc27260c370cbf9f795294f20e18abd24ada6 Parents: b877721 Author: Ian Maxon Authored: Mon Nov 9 14:29:46 2015 -0800 Committer: Ian Maxon Committed: Tue Nov 10 08:38:19 2015 -0800 ---------------------------------------------------------------------- .../numeric/issue_1166/issue_1166.1.query.aql | 21 ++++++++++++++++++++ .../results/numeric/issue_1166/issue_1166.1.adm | 3 +++ .../src/test/resources/runtimets/testsuite.xml | 5 +++++ asterix-docker/docker/Dockerfile | 4 ++-- .../functions/NumericAddDescriptor.java | 9 ++------- .../functions/NumericCaretDescriptor.java | 11 ++++------ .../functions/NumericDivideDescriptor.java | 8 ++++++-- .../functions/NumericMultiplyDescriptor.java | 17 ++-------------- .../functions/NumericSubDescriptor.java | 8 +------- 9 files changed, 46 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-app/src/test/resources/runtimets/queries/numeric/issue_1166/issue_1166.1.query.aql ---------------------------------------------------------------------- diff --git a/asterix-app/src/test/resources/runtimets/queries/numeric/issue_1166/issue_1166.1.query.aql b/asterix-app/src/test/resources/runtimets/queries/numeric/issue_1166/issue_1166.1.query.aql new file mode 100644 index 0000000..bd73cce --- /dev/null +++ b/asterix-app/src/test/resources/runtimets/queries/numeric/issue_1166/issue_1166.1.query.aql @@ -0,0 +1,21 @@ +/* + * 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. + */ +(-0.999 * 5) +(-1.001 * 5) +(-1 * 5) http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-app/src/test/resources/runtimets/results/numeric/issue_1166/issue_1166.1.adm ---------------------------------------------------------------------- diff --git a/asterix-app/src/test/resources/runtimets/results/numeric/issue_1166/issue_1166.1.adm b/asterix-app/src/test/resources/runtimets/results/numeric/issue_1166/issue_1166.1.adm new file mode 100644 index 0000000..06a5b6d --- /dev/null +++ b/asterix-app/src/test/resources/runtimets/results/numeric/issue_1166/issue_1166.1.adm @@ -0,0 +1,3 @@ +-4.995d +-5.004999999999999d +-5 http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-app/src/test/resources/runtimets/testsuite.xml ---------------------------------------------------------------------- diff --git a/asterix-app/src/test/resources/runtimets/testsuite.xml b/asterix-app/src/test/resources/runtimets/testsuite.xml index 563163b..4d35ac0 100644 --- a/asterix-app/src/test/resources/runtimets/testsuite.xml +++ b/asterix-app/src/test/resources/runtimets/testsuite.xml @@ -3887,6 +3887,11 @@ + + issue_1166 + + + multiply_int8 http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-docker/docker/Dockerfile ---------------------------------------------------------------------- diff --git a/asterix-docker/docker/Dockerfile b/asterix-docker/docker/Dockerfile index 9303bce..4e11474 100644 --- a/asterix-docker/docker/Dockerfile +++ b/asterix-docker/docker/Dockerfile @@ -21,7 +21,7 @@ MAINTAINER AsterixDB Team RUN echo 'LANG="en_US.UTF-8"' > /etc/sysconfig/i18n ;echo 'ZONE="America/Los_Angeles"' > /etc/sysconfig/clock ;cp -a /usr/share/zoneinfo/America/Los_Angeles /etc/localtime RUN echo "include_only=.us" >> /etc/yum/pluginconf.d/fastestmirror.conf -RUN yum install -y unzip java-1.7.0-openjdk openssh-server openssh-clients python-setuptools wget curl +RUN yum install -y unzip java-1.8.0-openjdk openssh-server openssh-clients python-setuptools wget curl RUN easy_install supervisor RUN mkdir /asterixdb COPY asterix-server*.zip . @@ -34,7 +34,7 @@ COPY fbu.adm /asterixdb/fbu.adm COPY fbm.adm /asterixdb/fbm.adm WORKDIR /asterixdb/bin -ENV JAVA_HOME /usr/lib/jvm/jre-1.7.0 +ENV JAVA_HOME /usr/lib/jvm/jre-1.8.0 ENV JAVA_OPTS -Xmx1536m EXPOSE 19001 19002 8888 19003 50031 http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericAddDescriptor.java ---------------------------------------------------------------------- diff --git a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericAddDescriptor.java b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericAddDescriptor.java index 226dd8d..265f68e 100644 --- a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericAddDescriptor.java +++ b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericAddDescriptor.java @@ -24,6 +24,7 @@ import org.apache.asterix.om.functions.IFunctionDescriptor; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.api.exceptions.HyracksDataException; +import java.lang.Math; public class NumericAddDescriptor extends AbstractNumericArithmeticEval { @@ -41,13 +42,7 @@ public class NumericAddDescriptor extends AbstractNumericArithmeticEval { @Override protected long evaluateInteger(long x, long y) throws HyracksDataException { - long z = x + y; - if (x > 0) { - if (y > 0 && z < 0) - throw new ArithmeticException("Overflow adding " + x + " + " + y); - } else if (y < 0 && z > 0) - throw new ArithmeticException("Underflow adding " + x + " + " + y); - return z; + return Math.addExact(x, y); } @Override http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java ---------------------------------------------------------------------- diff --git a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java index 2412ddc..60f1233 100644 --- a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java +++ b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericCaretDescriptor.java @@ -18,6 +18,7 @@ */ package org.apache.asterix.runtime.evaluators.functions; +import com.google.common.math.LongMath; import org.apache.asterix.om.functions.AsterixBuiltinFunctions; import org.apache.asterix.om.functions.IFunctionDescriptor; import org.apache.asterix.om.functions.IFunctionDescriptorFactory; @@ -40,14 +41,10 @@ public class NumericCaretDescriptor extends AbstractNumericArithmeticEval { */ @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { - double result = Math.pow(lhs, rhs); - if (result > Long.MAX_VALUE) { - throw new ArithmeticException("Overflow of caret operation: " + lhs + " ^ " + rhs); + if(rhs > Integer.MAX_VALUE){ + throw new ArithmeticException("Exponent cannot be larger than 2^31-1"); } - if (result < Long.MIN_VALUE) { - throw new ArithmeticException("Underflow of caret operation: " + lhs + " ^ " + rhs); - } - return (long) result; + return LongMath.checkedPow(lhs, (int) rhs); } /* (non-Javadoc) http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java ---------------------------------------------------------------------- diff --git a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java index b4265c7..1c5cb9c 100644 --- a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java +++ b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericDivideDescriptor.java @@ -41,8 +41,12 @@ public class NumericDivideDescriptor extends AbstractNumericArithmeticEval { @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { - if (rhs == 0) - throw new HyracksDataException("Divide by Zero."); + if (rhs == 0) { + throw new ArithmeticException("Division by Zero."); + } + if ( (lhs == Long.MIN_VALUE) && (rhs == -1L) ) { + throw new ArithmeticException(("Overflow in integer division")); + } return lhs / rhs; } http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericMultiplyDescriptor.java ---------------------------------------------------------------------- diff --git a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericMultiplyDescriptor.java b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericMultiplyDescriptor.java index 4bb29e7..c3f6580 100644 --- a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericMultiplyDescriptor.java +++ b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericMultiplyDescriptor.java @@ -24,6 +24,7 @@ import org.apache.asterix.om.functions.IFunctionDescriptorFactory; import org.apache.hyracks.algebricks.common.exceptions.NotImplementedException; import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier; import org.apache.hyracks.api.exceptions.HyracksDataException; +import java.lang.Math; public class NumericMultiplyDescriptor extends AbstractNumericArithmeticEval { @@ -41,25 +42,11 @@ public class NumericMultiplyDescriptor extends AbstractNumericArithmeticEval { @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { - int signLhs = lhs > 0 ? 1 : (lhs < 0 ? -1 : 0); - int signRhs = rhs > 0 ? 1 : (rhs < 0 ? -1 : 0); - long maximum = signLhs == signRhs ? Long.MAX_VALUE : Long.MIN_VALUE; - - if (lhs != 0 && (rhs > 0 && rhs > maximum / lhs || rhs < 0 && rhs < maximum / lhs)) - throw new HyracksDataException("Overflow Happened."); - - return lhs * rhs; + return Math.multiplyExact(lhs, rhs); } @Override protected double evaluateDouble(double lhs, double rhs) throws HyracksDataException { - int signLhs = lhs > 0 ? 1 : (lhs < 0 ? -1 : 0); - int signRhs = rhs > 0 ? 1 : (rhs < 0 ? -1 : 0); - double maximum = signLhs == signRhs ? Double.MAX_VALUE : -Double.MAX_VALUE; - - if (lhs != 0 && (rhs > 0 && rhs > maximum / lhs || rhs < 0 && rhs < maximum / lhs)) - throw new HyracksDataException("Overflow Happened."); - return lhs * rhs; } http://git-wip-us.apache.org/repos/asf/incubator-asterixdb/blob/89bbc272/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericSubDescriptor.java ---------------------------------------------------------------------- diff --git a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericSubDescriptor.java b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericSubDescriptor.java index 6c67b53..2cb4411 100644 --- a/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericSubDescriptor.java +++ b/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/NumericSubDescriptor.java @@ -44,13 +44,7 @@ public class NumericSubDescriptor extends AbstractNumericArithmeticEval { */ @Override protected long evaluateInteger(long lhs, long rhs) throws HyracksDataException { - long res = lhs - rhs; - if (lhs > 0) { - if (rhs < 0 && res < 0) - throw new HyracksDataException("Overflow adding " + lhs + " + " + rhs); - } else if (rhs > 0 && res > 0) - throw new HyracksDataException("Underflow adding " + lhs + " + " + rhs); - return res; + return Math.subtractExact(lhs, rhs); } /* (non-Javadoc)