Improve handling of INT_MIN / -1 and related cases.

Some platforms throw an exception for this division, rather than returning
a necessarily-overflowed result.  Since we were testing for overflow after
the fact, an exception isn't nice.  We can avoid the problem by treating
division by -1 as negation.

Add some regression tests so that we'll find out if any compilers try to
optimize away the overflow check conditions.

This ought to be back-patched, but I'm going to see what the buildfarm
reports about the regression tests first.

Per discussion with Xi Wang, though this is different from the patch he
submitted.

diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 9f752edc2922643ef6d7fca11f8db0753a152661..3a6f587d069c99a3dc7ae8bfbf9fd8d496d22ab4 100644 (file)
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -681,18 +681,6 @@ int4mul(PG_FUNCTION_ARGS)
    int32       arg2 = PG_GETARG_INT32(1);
    int32       result;
 
-#ifdef WIN32
-
-   /*
-    * Win32 doesn't throw a catchable exception for SELECT -2147483648 *
-    * (-1);  -- INT_MIN
-    */
-   if (arg2 == -1 && arg1 == INT_MIN)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("integer out of range")));
-#endif
-
    result = arg1 * arg2;
 
    /*
@@ -709,7 +697,8 @@ int4mul(PG_FUNCTION_ARGS)
    if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX &&
          arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) &&
        arg2 != 0 &&
-       (result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
+       ((arg2 == -1 && arg1 < 0 && result < 0) ||
+        result / arg2 != arg1))
        ereport(ERROR,
                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                 errmsg("integer out of range")));
@@ -732,30 +721,27 @@ int4div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-#ifdef WIN32
-
    /*
-    * Win32 doesn't throw a catchable exception for SELECT -2147483648 /
-    * (-1); -- INT_MIN
+    * INT_MIN / -1 is problematic, since the result can't be represented on a
+    * two's-complement machine.  Some machines produce INT_MIN, some produce
+    * zero, some throw an exception.  We can dodge the problem by recognizing
+    * that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 == INT_MIN)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("integer out of range")));
-#endif
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for INT_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("integer out of range")));
+       PG_RETURN_INT32(result);
+   }
+
+   /* No overflow is possible */
 
    result = arg1 / arg2;
 
-   /*
-    * Overflow check.  The only possible overflow case is for arg1 = INT_MIN,
-    * arg2 = -1, where the correct result is -INT_MIN, which can't be
-    * represented on a two's-complement machine.  Most machines produce
-    * INT_MIN but it seems some produce zero.
-    */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("integer out of range")));
    PG_RETURN_INT32(result);
 }
 
@@ -877,18 +863,27 @@ int2div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-   result = arg1 / arg2;
-
    /*
-    * Overflow check.  The only possible overflow case is for arg1 =
-    * SHRT_MIN, arg2 = -1, where the correct result is -SHRT_MIN, which can't
-    * be represented on a two's-complement machine.  Most machines produce
-    * SHRT_MIN but it seems some produce zero.
+    * SHRT_MIN / -1 is problematic, since the result can't be represented on
+    * a two's-complement machine.  Some machines produce SHRT_MIN, some
+    * produce zero, some throw an exception.  We can dodge the problem by
+    * recognizing that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("smallint out of range")));
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for SHRT_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("smallint out of range")));
+       PG_RETURN_INT16(result);
+   }
+
+   /* No overflow is possible */
+
+   result = arg1 / arg2;
+
    PG_RETURN_INT16(result);
 }
 
@@ -1065,18 +1060,27 @@ int42div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-   result = arg1 / arg2;
-
    /*
-    * Overflow check.  The only possible overflow case is for arg1 = INT_MIN,
-    * arg2 = -1, where the correct result is -INT_MIN, which can't be
-    * represented on a two's-complement machine.  Most machines produce
-    * INT_MIN but it seems some produce zero.
+    * INT_MIN / -1 is problematic, since the result can't be represented on a
+    * two's-complement machine.  Some machines produce INT_MIN, some produce
+    * zero, some throw an exception.  We can dodge the problem by recognizing
+    * that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("integer out of range")));
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for INT_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("integer out of range")));
+       PG_RETURN_INT32(result);
+   }
+
+   /* No overflow is possible */
+
+   result = arg1 / arg2;
+
    PG_RETURN_INT32(result);
 }
 

diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 59c110b0b302e17bda116a6a89793eac0a8d6bc4..c4cb1f2eff785c97a1ade85bf323b2e9951b4f2d 100644 (file)
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -574,7 +574,8 @@ int8mul(PG_FUNCTION_ARGS)
    if (arg1 != (int64) ((int32) arg1) || arg2 != (int64) ((int32) arg2))
    {
        if (arg2 != 0 &&
-           (result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
+           ((arg2 == -1 && arg1 < 0 && result < 0) ||
+            result / arg2 != arg1))
            ereport(ERROR,
                    (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                     errmsg("bigint out of range")));
@@ -598,18 +599,27 @@ int8div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-   result = arg1 / arg2;
-
    /*
-    * Overflow check.  The only possible overflow case is for arg1 =
-    * INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
-    * can't be represented on a two's-complement machine.  Most machines
-    * produce INT64_MIN but it seems some produce zero.
+    * INT64_MIN / -1 is problematic, since the result can't be represented on
+    * a two's-complement machine.  Some machines produce INT64_MIN, some
+    * produce zero, some throw an exception.  We can dodge the problem by
+    * recognizing that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("bigint out of range")));
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for INT64_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("bigint out of range")));
+       PG_RETURN_INT64(result);
+   }
+
+   /* No overflow is possible */
+
+   result = arg1 / arg2;
+
    PG_RETURN_INT64(result);
 }
 
@@ -838,18 +848,27 @@ int84div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-   result = arg1 / arg2;
-
    /*
-    * Overflow check.  The only possible overflow case is for arg1 =
-    * INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
-    * can't be represented on a two's-complement machine.  Most machines
-    * produce INT64_MIN but it seems some produce zero.
+    * INT64_MIN / -1 is problematic, since the result can't be represented on
+    * a two's-complement machine.  Some machines produce INT64_MIN, some
+    * produce zero, some throw an exception.  We can dodge the problem by
+    * recognizing that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("bigint out of range")));
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for INT64_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("bigint out of range")));
+       PG_RETURN_INT64(result);
+   }
+
+   /* No overflow is possible */
+
+   result = arg1 / arg2;
+
    PG_RETURN_INT64(result);
 }
 
@@ -1026,18 +1045,27 @@ int82div(PG_FUNCTION_ARGS)
        PG_RETURN_NULL();
    }
 
-   result = arg1 / arg2;
-
    /*
-    * Overflow check.  The only possible overflow case is for arg1 =
-    * INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
-    * can't be represented on a two's-complement machine.  Most machines
-    * produce INT64_MIN but it seems some produce zero.
+    * INT64_MIN / -1 is problematic, since the result can't be represented on
+    * a two's-complement machine.  Some machines produce INT64_MIN, some
+    * produce zero, some throw an exception.  We can dodge the problem by
+    * recognizing that division by -1 is the same as negation.
     */
-   if (arg2 == -1 && arg1 < 0 && result <= 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                errmsg("bigint out of range")));
+   if (arg2 == -1)
+   {
+       result = -arg1;
+       /* overflow check (needed for INT64_MIN) */
+       if (arg1 != 0 && SAMESIGN(result, arg1))
+           ereport(ERROR,
+                   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                    errmsg("bigint out of range")));
+       PG_RETURN_INT64(result);
+   }
+
+   /* No overflow is possible */
+
+   result = arg1 / arg2;
+
    PG_RETURN_INT64(result);
 }
 

diff --git a/src/test/regress/expected/int2.out b/src/test/regress/expected/int2.out
index 021d476822ca1dd2c7529c75acc6b908d2f7e647..53b484f718c67b7fdcc082445724446a51ff7233 100644 (file)
--- a/src/test/regress/expected/int2.out
+++ b/src/test/regress/expected/int2.out
@@ -255,3 +255,14 @@ SELECT ((-1::int2<<15)+1::int2)::text;
  -32767
 (1 row)
 
+-- check sane handling of INT16_MIN overflow cases
+SELECT (-32768)::int2 * (-1)::int2;
+ERROR:  smallint out of range
+SELECT (-32768)::int2 / (-1)::int2;
+ERROR:  smallint out of range
+SELECT (-32768)::int2 % (-1)::int2;
+ ?column? 
+----------
+        0
+(1 row)
+

diff --git a/src/test/regress/expected/int4.out b/src/test/regress/expected/int4.out
index 8f780240ae781108c715e4936bb65a1e1db08595..fcb14e3855e0298196028b334db48e501b87ad96 100644 (file)
--- a/src/test/regress/expected/int4.out
+++ b/src/test/regress/expected/int4.out
@@ -342,3 +342,24 @@ SELECT ((-1::int4<<31)+1)::text;
  -2147483647
 (1 row)
 
+-- check sane handling of INT_MIN overflow cases
+SELECT (-2147483648)::int4 * (-1)::int4;
+ERROR:  integer out of range
+SELECT (-2147483648)::int4 / (-1)::int4;
+ERROR:  integer out of range
+SELECT (-2147483648)::int4 % (-1)::int4;
+ ?column? 
+----------
+        0
+(1 row)
+
+SELECT (-2147483648)::int4 * (-1)::int2;
+ERROR:  integer out of range
+SELECT (-2147483648)::int4 / (-1)::int2;
+ERROR:  integer out of range
+SELECT (-2147483648)::int4 % (-1)::int2;
+ ?column? 
+----------
+        0
+(1 row)
+

diff --git a/src/test/regress/expected/int8-exp-three-digits.out b/src/test/regress/expected/int8-exp-three-digits.out
index b523bfcc01b28dc573d3127f727017e7917ed987..a1c70ed3e8d3a6c6bcc30587c59abdb3a1c893f4 100644 (file)
--- a/src/test/regress/expected/int8-exp-three-digits.out
+++ b/src/test/regress/expected/int8-exp-three-digits.out
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
  -9223372036854775807
 (1 row)
 
+-- check sane handling of INT64_MIN overflow cases
+SELECT (-9223372036854775808)::int8 * (-1)::int8;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int8;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int8;
+ ?column? 
+----------
+        0
+(1 row)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int4;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int4;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int4;
+ ?column? 
+----------
+        0
+(1 row)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int2;
+ ?column? 
+----------
+        0
+(1 row)
+

diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out
index 811d6a5520068966720569aa55acb539bbed747c..e79c3a8af95c8372ebd8526f26fc504097868b42 100644 (file)
--- a/src/test/regress/expected/int8.out
+++ b/src/test/regress/expected/int8.out
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
  -9223372036854775807
 (1 row)
 
+-- check sane handling of INT64_MIN overflow cases
+SELECT (-9223372036854775808)::int8 * (-1)::int8;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int8;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int8;
+ ?column? 
+----------
+        0
+(1 row)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int4;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int4;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int4;
+ ?column? 
+----------
+        0
+(1 row)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+ERROR:  bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int2;
+ ?column? 
+----------
+        0
+(1 row)
+

diff --git a/src/test/regress/sql/int2.sql b/src/test/regress/sql/int2.sql
index f11eb283c61d8a24c94c4597a15341d5608490c3..bacfbb24ac2bd6387c13deca72f7d0ada64eba4b 100644 (file)
--- a/src/test/regress/sql/int2.sql
+++ b/src/test/regress/sql/int2.sql
@@ -87,3 +87,8 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
 -- corner cases
 SELECT (-1::int2<<15)::text;
 SELECT ((-1::int2<<15)+1::int2)::text;
+
+-- check sane handling of INT16_MIN overflow cases
+SELECT (-32768)::int2 * (-1)::int2;
+SELECT (-32768)::int2 / (-1)::int2;
+SELECT (-32768)::int2 % (-1)::int2;

diff --git a/src/test/regress/sql/int4.sql b/src/test/regress/sql/int4.sql
index ffae7ce4cb470f89265e368877acd70b33b6fd5e..1843a6d33bc86e774190f00d23cf90cb3248680e 100644 (file)
--- a/src/test/regress/sql/int4.sql
+++ b/src/test/regress/sql/int4.sql
@@ -127,3 +127,11 @@ SELECT (2 + 2) / 2 AS two;
 -- corner case
 SELECT (-1::int4<<31)::text;
 SELECT ((-1::int4<<31)+1)::text;
+
+-- check sane handling of INT_MIN overflow cases
+SELECT (-2147483648)::int4 * (-1)::int4;
+SELECT (-2147483648)::int4 / (-1)::int4;
+SELECT (-2147483648)::int4 % (-1)::int4;
+SELECT (-2147483648)::int4 * (-1)::int2;
+SELECT (-2147483648)::int4 / (-1)::int2;
+SELECT (-2147483648)::int4 % (-1)::int2;

diff --git a/src/test/regress/sql/int8.sql b/src/test/regress/sql/int8.sql
index 27e0696b32f9c05eb0b98b78e87cf24d3bb61f70..2f7f30c91d384f6ffdf21db9ef19a4d748979677 100644 (file)
--- a/src/test/regress/sql/int8.sql
+++ b/src/test/regress/sql/int8.sql
@@ -194,3 +194,14 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
 -- corner case
 SELECT (-1::int8<<63)::text;
 SELECT ((-1::int8<<63)+1)::text;
+
+-- check sane handling of INT64_MIN overflow cases
+SELECT (-9223372036854775808)::int8 * (-1)::int8;
+SELECT (-9223372036854775808)::int8 / (-1)::int8;
+SELECT (-9223372036854775808)::int8 % (-1)::int8;
+SELECT (-9223372036854775808)::int8 * (-1)::int4;
+SELECT (-9223372036854775808)::int8 / (-1)::int4;
+SELECT (-9223372036854775808)::int8 % (-1)::int4;
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+SELECT (-9223372036854775808)::int8 % (-1)::int2;