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.

Back-patch of commit 1f7cb5c30983752ff8de833de30afcaee63536d0.

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 6496d1367f5dea8e8c1485644eb0586c1a8741f5..1a82321ae1aa259d9762eec13c746c11e717a028 100644 (file)
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -671,18 +671,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;
 
        /*
@@ -699,7 +687,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")));
@@ -722,30 +711,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);
 }
 
@@ -867,18 +853,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);
 }
 
@@ -1055,18 +1050,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 5eed95d298ab7b24618b96a8c38223d7404dfea3..c0b602cb5aedda5db04cbfcd34919d9acacdf2e8 100644 (file)
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -583,7 +583,8 @@ int8mul(PG_FUNCTION_ARGS)
 #endif
        {
                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")));
@@ -607,18 +608,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);
 }
 
@@ -849,18 +859,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);
 }
 
@@ -1037,18 +1056,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 61ac956cb74cb935622313c803e54e344e4f1aeb..f4af51f46e112e11d244e77920217e8fd40707b5 100644 (file)
--- a/src/test/regress/expected/int2.out
+++ b/src/test/regress/expected/int2.out
@@ -244,3 +244,14 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
       | -32767 | -16383
 (5 rows)
 
+-- 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 a21bbda2c9fc633656c292745d6418b2991cbd7c..3cfd1d0cd6326c453f3ddac6b4a554744827d0df 100644 (file)
--- a/src/test/regress/expected/int4.out
+++ b/src/test/regress/expected/int4.out
@@ -331,3 +331,24 @@ SELECT (2 + 2) / 2 AS two;
    2
 (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 62d975bce192ca9d42880eac13c8cfc639f04a35..da970f55e898407331fa28569956304ec07190f8 100644 (file)
--- a/src/test/regress/expected/int8-exp-three-digits.out
+++ b/src/test/regress/expected/int8-exp-three-digits.out
@@ -802,3 +802,34 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
  4567890123456799
 (6 rows)
 
+-- 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 c8e2dad683389ee542bf0e41e2b0608e3dd3d105..7b735560a81461475893d459d114b4b7d0655c50 100644 (file)
--- a/src/test/regress/expected/int8.out
+++ b/src/test/regress/expected/int8.out
@@ -802,3 +802,34 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
  4567890123456799
 (6 rows)
 
+-- 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 65c89e4abdd0b03dd0c8eb093bfbbac88d6cb095..b185f16c0aa395904507b6dd517e7750a5af6399 100644 (file)
--- a/src/test/regress/sql/int2.sql
+++ b/src/test/regress/sql/int2.sql
@@ -86,3 +86,7 @@ SELECT '' AS five, i.f1, i.f1 / int2 '2' AS x FROM INT2_TBL i;
 
 SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
 
+-- 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 5212c687952724d26a19a25717a8fc7f7c75a1d2..9c24188dc0d74f57c672d3d03736bb488bb98859 100644 (file)
--- a/src/test/regress/sql/int4.sql
+++ b/src/test/regress/sql/int4.sql
@@ -125,3 +125,11 @@ SELECT 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 AS ten;
 SELECT 2 + 2 / 2 AS three;
 
 SELECT (2 + 2) / 2 AS two;
+
+-- 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 648563c37446ef17af8ed8a7f279e80c3a8c7d18..9e951a52bdd8e91afd0814c61321e9a78e40daa7 100644 (file)
--- a/src/test/regress/sql/int8.sql
+++ b/src/test/regress/sql/int8.sql
@@ -190,3 +190,14 @@ SELECT q1, q1 << 2 AS "shl", q1 >> 3 AS "shr" FROM INT8_TBL;
 SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8);
 SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8, 0);
 SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::int8, 2);
+
+-- 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;