Merge pull request #1134 from katzyn/misc

Detect possible overflow in integer division and optimize some code

h2/src/main/org/h2/command/Parser.java

@@ -3259,7 +3259,7 @@ public class Parser {
                     r = ValueExpression.get(ValueInt.get(Integer.MIN_VALUE));
                 } else if (r.getType() == Value.DECIMAL &&
                         r.getValue(session).getBigDecimal()
-                                .compareTo(ValueLong.MIN_BD) == 0) {
+                                .compareTo(Value.MIN_LONG_DECIMAL) == 0) {
                     // convert Long.MIN_VALUE to type 'long'
                     // (Long.MAX_VALUE+1 is of type 'decimal')
                     r = ValueExpression.get(ValueLong.MIN);

h2/src/main/org/h2/command/ddl/DropSchema.java

@@ -51,7 +51,7 @@ public class DropSchema extends DefineCommand {
             }
             if (dropAction == ConstraintActionType.RESTRICT && !schema.isEmpty()) {
                 StatementBuilder buff = new StatementBuilder();
-                for (SchemaObject object : schema.getAll()) {
+                for (SchemaObject object : schema.getAll(null)) {
                     buff.appendExceptFirst(", ");
                     buff.append(object.getName());
                 }

h2/src/main/org/h2/engine/Database.java

@@ -1564,7 +1564,7 @@ public class Database implements DataHandler {
         initMetaTables();
         ArrayList<SchemaObject> list = new ArrayList<>();
         for (Schema schema : schemas.values()) {
-            list.addAll(schema.getAll());
+            schema.getAll(list);
         }
         return list;
     }
@@ -1581,7 +1581,7 @@ public class Database implements DataHandler {
         }
         ArrayList<SchemaObject> list = new ArrayList<>();
         for (Schema schema : schemas.values()) {
-            list.addAll(schema.getAll(type));
+            schema.getAll(type, list);
         }
         return list;
     }

h2/src/main/org/h2/schema/Schema.java

@@ -6,6 +6,7 @@
 package org.h2.schema;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -591,30 +592,46 @@ public class Schema extends DbObjectBase {
     /**
      * Get all objects.
      *
-     * @return a (possible empty) list of all objects
+     * @param addTo
+     *                  list to add objects to, or {@code null} to allocate a new
+     *                  list
+     * @return the specified list with added objects, or a new (possibly empty) list
+     *         with all objects
      */
-    public ArrayList<SchemaObject> getAll() {
-        ArrayList<SchemaObject> all = Utils.newSmallArrayList();
-        all.addAll(getMap(DbObject.TABLE_OR_VIEW).values());
-        all.addAll(getMap(DbObject.SYNONYM).values());
-        all.addAll(getMap(DbObject.SEQUENCE).values());
-        all.addAll(getMap(DbObject.INDEX).values());
-        all.addAll(getMap(DbObject.TRIGGER).values());
-        all.addAll(getMap(DbObject.CONSTRAINT).values());
-        all.addAll(getMap(DbObject.CONSTANT).values());
-        all.addAll(getMap(DbObject.FUNCTION_ALIAS).values());
-        return all;
+    public ArrayList<SchemaObject> getAll(ArrayList<SchemaObject> addTo) {
+        if (addTo == null) {
+            addTo = Utils.newSmallArrayList();
+        }
+        addTo.addAll(tablesAndViews.values());
+        addTo.addAll(synonyms.values());
+        addTo.addAll(sequences.values());
+        addTo.addAll(indexes.values());
+        addTo.addAll(triggers.values());
+        addTo.addAll(constraints.values());
+        addTo.addAll(constants.values());
+        addTo.addAll(functions.values());
+        return addTo;
     }
 
     /**
      * Get all objects of the given type.
      *
-     * @param type the object type
-     * @return a (possible empty) list of all objects
-     */
-    public ArrayList<SchemaObject> getAll(int type) {
-        Map<String, SchemaObject> map = getMap(type);
-        return new ArrayList<>(map.values());
+     * @param type
+     *                  the object type
+     * @param addTo
+     *                  list to add objects to, or {@code null} to allocate a new
+     *                  list
+     * @return the specified list with added objects, or a new (possibly empty) list
+     *         with objects of the given type
+     */
+    public ArrayList<SchemaObject> getAll(int type, ArrayList<SchemaObject> addTo) {
+        Collection<SchemaObject> values = getMap(type).values();
+        if (addTo != null) {
+            addTo.addAll(values);
+        } else {
+            addTo = new ArrayList<>(values);
+        }
+        return addTo;
     }
 
     /**

h2/src/main/org/h2/value/Value.java

@@ -179,11 +179,14 @@ public abstract class Value {
      */
     public static final int TYPE_COUNT = ENUM;
 
-    private static SoftReference<Value[]> softCache =
-            new SoftReference<>(null);
+    private static SoftReference<Value[]> softCache;
     private static final BigDecimal MAX_LONG_DECIMAL =
             BigDecimal.valueOf(Long.MAX_VALUE);
-    private static final BigDecimal MIN_LONG_DECIMAL =
+
+    /**
+     * The smallest Long value, as a BigDecimal.
+     */
+    public static final BigDecimal MIN_LONG_DECIMAL =
             BigDecimal.valueOf(Long.MIN_VALUE);
 
     /**
@@ -396,11 +399,8 @@ public abstract class Value {
     static Value cache(Value v) {
         if (SysProperties.OBJECT_CACHE) {
             int hash = v.hashCode();
-            if (softCache == null) {
-                softCache = new SoftReference<>(null);
-            }
-            Value[] cache = softCache.get();
-            if (cache == null) {
+            Value[] cache;
+            if (softCache == null || (cache = softCache.get()) == null) {
                 cache = new Value[SysProperties.OBJECT_CACHE_SIZE];
                 softCache = new SoftReference<>(cache);
             }

h2/src/main/org/h2/value/ValueByte.java

@@ -75,7 +75,7 @@ public class ValueByte extends Value {
         if (other.value == 0) {
             throw DbException.get(ErrorCode.DIVISION_BY_ZERO_1, getSQL());
         }
-        return ValueByte.get((byte) (value / other.value));
+        return checkRange(value / other.value);
     }
 
     @Override

h2/src/main/org/h2/value/ValueInt.java

@@ -100,11 +100,15 @@ public class ValueInt extends Value {
 
     @Override
     public Value divide(Value v) {
-        ValueInt other = (ValueInt) v;
-        if (other.value == 0) {
+        int y = ((ValueInt) v).value;
+        if (y == 0) {
             throw DbException.get(ErrorCode.DIVISION_BY_ZERO_1, getSQL());
         }
-        return ValueInt.get(value / other.value);
+        int x = value;
+        if (x == Integer.MIN_VALUE && y == -1) {
+            throw DbException.get(ErrorCode.NUMERIC_VALUE_OUT_OF_RANGE_1, "2147483648");
+        }
+        return ValueInt.get(x / y);
     }
 
     @Override

h2/src/main/org/h2/value/ValueLong.java

@@ -5,7 +5,6 @@
  */
 package org.h2.value;
 
-import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
@@ -33,11 +32,6 @@ public class ValueLong extends Value {
      */
     public static final BigInteger MAX_BI = BigInteger.valueOf(Long.MAX_VALUE);
 
-    /**
-     * The smallest Long value, as a BigDecimal.
-     */
-    public static final BigDecimal MIN_BD = BigDecimal.valueOf(Long.MIN_VALUE);
-
     /**
      * The precision in digits.
      */
@@ -49,7 +43,6 @@ public class ValueLong extends Value {
      */
     public static final int DISPLAY_SIZE = 20;
 
-    private static final BigInteger MIN_BI = BigInteger.valueOf(Long.MIN_VALUE);
     private static final int STATIC_SIZE = 100;
     private static final ValueLong[] STATIC_CACHE;
 
@@ -116,41 +109,33 @@ public class ValueLong extends Value {
         return add(other.negate());
     }
 
-    private static boolean isInteger(long a) {
-        return a >= Integer.MIN_VALUE && a <= Integer.MAX_VALUE;
-    }
-
     @Override
     public Value multiply(Value v) {
-        ValueLong other = (ValueLong) v;
-        long result = value * other.value;
-        if (value == 0 || value == 1 || other.value == 0 || other.value == 1) {
-            return ValueLong.get(result);
-        }
-        if (isInteger(value) && isInteger(other.value)) {
-            return ValueLong.get(result);
-        }
-        // just checking one case is not enough: Long.MIN_VALUE * -1
-        // probably this is correct but I'm not sure
-        // if (result / value == other.value && result / other.value == value) {
-        //    return ValueLong.get(result);
-        //}
-        BigInteger bv = BigInteger.valueOf(value);
-        BigInteger bo = BigInteger.valueOf(other.value);
-        BigInteger br = bv.multiply(bo);
-        if (br.compareTo(MIN_BI) < 0 || br.compareTo(MAX_BI) > 0) {
+        long x = value;
+        long y = ((ValueLong) v).value;
+        long result = x * y;
+        // Check whether numbers are large enough to overflow and second value != 0
+        if ((Math.abs(x) | Math.abs(y)) >>> 31 != 0 && y != 0
+                // Check with division
+                && (result / y != x
+                // Also check the special condition that is not handled above
+                || x == Long.MIN_VALUE && y == -1)) {
             throw getOverflow();
         }
-        return ValueLong.get(br.longValue());
+        return ValueLong.get(result);
     }
 
     @Override
     public Value divide(Value v) {
-        ValueLong other = (ValueLong) v;
-        if (other.value == 0) {
+        long y = ((ValueLong) v).value;
+        if (y == 0) {
             throw DbException.get(ErrorCode.DIVISION_BY_ZERO_1, getSQL());
         }
-        return ValueLong.get(value / other.value);
+        long x = value;
+        if (x == Long.MIN_VALUE && y == -1) {
+            throw getOverflow();
+        }
+        return ValueLong.get(x / y);
     }
 
     @Override

h2/src/main/org/h2/value/ValueShort.java

@@ -75,7 +75,7 @@ public class ValueShort extends Value {
         if (other.value == 0) {
             throw DbException.get(ErrorCode.DIVISION_BY_ZERO_1, getSQL());
         }
-        return ValueShort.get((short) (value / other.value));
+        return checkRange(value / other.value);
     }
 
     @Override

h2/src/test/org/h2/test/scripts/datatypes/bigint.sql

@@ -2,3 +2,52 @@
 -- and the EPL 1.0 (http://h2database.com/html/license.html).
 -- Initial Developer: H2 Group
 --
+
+-- Multiplication
+
+SELECT CAST(-4294967296 AS BIGINT) * CAST (2147483648 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(4294967296 AS BIGINT) * CAST (-2147483648 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(-2147483648 AS BIGINT) * CAST (4294967296 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(2147483648 AS BIGINT) * CAST (-4294967296 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(4294967296 AS BIGINT) * CAST (2147483648 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+SELECT CAST(-4294967296 AS BIGINT) * CAST (-2147483648 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+SELECT CAST(2147483648 AS BIGINT) * CAST (4294967296 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+SELECT CAST(-2147483648 AS BIGINT) * CAST (-4294967296 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+SELECT CAST(-9223372036854775808 AS BIGINT) * CAST(1 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(-9223372036854775808 AS BIGINT) * CAST(-1 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+SELECT CAST(1 AS BIGINT) * CAST(-9223372036854775808 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(-1 AS BIGINT) * CAST(-9223372036854775808 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1
+
+-- Division
+
+SELECT CAST(1 AS BIGINT) / CAST(0 AS BIGINT);
+> exception DIVISION_BY_ZERO_1
+
+SELECT CAST(-9223372036854775808 AS BIGINT) / CAST(1 AS BIGINT);
+>> -9223372036854775808
+
+SELECT CAST(-9223372036854775808 AS BIGINT) / CAST(-1 AS BIGINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1

h2/src/test/org/h2/test/scripts/datatypes/int.sql

@@ -2,3 +2,14 @@
 -- and the EPL 1.0 (http://h2database.com/html/license.html).
 -- Initial Developer: H2 Group
 --
+
+-- Division
+
+SELECT CAST(1 AS INT) / CAST(0 AS INT);
+> exception DIVISION_BY_ZERO_1
+
+SELECT CAST(-2147483648 AS INT) / CAST(1 AS INT);
+>> -2147483648
+
+SELECT CAST(-2147483648 AS INT) / CAST(-1 AS INT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1

h2/src/test/org/h2/test/scripts/datatypes/smallint.sql

@@ -2,3 +2,14 @@
 -- and the EPL 1.0 (http://h2database.com/html/license.html).
 -- Initial Developer: H2 Group
 --
+
+-- Division
+
+SELECT CAST(1 AS SMALLINT) / CAST(0 AS SMALLINT);
+> exception DIVISION_BY_ZERO_1
+
+SELECT CAST(-32768 AS SMALLINT) / CAST(1 AS SMALLINT);
+>> -32768
+
+SELECT CAST(-32768 AS SMALLINT) / CAST(-1 AS SMALLINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1

h2/src/test/org/h2/test/scripts/datatypes/tinyint.sql

@@ -2,3 +2,14 @@
 -- and the EPL 1.0 (http://h2database.com/html/license.html).
 -- Initial Developer: H2 Group
 --
+
+-- Division
+
+SELECT CAST(1 AS TINYINT) / CAST(0 AS TINYINT);
+> exception DIVISION_BY_ZERO_1
+
+SELECT CAST(-128 AS TINYINT) / CAST(1 AS TINYINT);
+>> -128
+
+SELECT CAST(-128 AS TINYINT) / CAST(-1 AS TINYINT);
+> exception NUMERIC_VALUE_OUT_OF_RANGE_1