I'm working on a patch to detect overflow in the integer-arithmetic
operators.  The first stage, covering the basic int4 operators, is
attached if anyone wants to comment on details.  A couple of general
questions though:

1. Does anyone object to applying this for 8.0?  I think we already had
consensus that it's a good idea, but if not now's the time to speak up.
(There are a couple of regression tests that fail and will need to be
adjusted, if that influences anyone's thinking.)

2. For the int2 and int8 operators, should we stick to a one-size-fits-all
message "integer out of range", or be more specific: "smallint out of
range" and "bigint out of range"?  The existing messages are not
completely consistent about this.  I'm inclined to go with mentioning
the specific type but I'm not dead set on it.

                        regards, tom lane


*** src/backend/utils/adt/int.c.orig    Sun Aug 29 09:55:24 2004
--- src/backend/utils/adt/int.c Sun Oct  3 15:20:13 2004
***************
*** 45,50 ****
--- 45,52 ----
  #define SHRT_MIN (-0x8000)
  #endif
  
+ #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0))
+ 
  typedef struct
  {
        int32           current;
***************
*** 601,608 ****
  int4um(PG_FUNCTION_ARGS)
  {
        int32           arg = PG_GETARG_INT32(0);
  
!       PG_RETURN_INT32(-arg);
  }
  
  Datum
--- 604,618 ----
  int4um(PG_FUNCTION_ARGS)
  {
        int32           arg = PG_GETARG_INT32(0);
+       int32           result;
  
!       result = -arg;
!       /* overflow check (needed for INT_MIN) */
!       if (arg != 0 && SAMESIGN(result, arg))
!               ereport(ERROR,
!                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                                errmsg("integer out of range")));
!       PG_RETURN_INT32(result);
  }
  
  Datum
***************
*** 618,625 ****
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
  
!       PG_RETURN_INT32(arg1 + arg2);
  }
  
  Datum
--- 628,646 ----
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
+       int32           result;
  
!       result = arg1 + arg2;
!       /*
!        * Overflow check.  If the inputs are of different signs then their sum
!        * cannot overflow.  If the inputs are of the same sign, their sum
!        * had better be that sign too.
!        */
!       if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1))
!               ereport(ERROR,
!                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                                errmsg("integer out of range")));
!       PG_RETURN_INT32(result);
  }
  
  Datum
***************
*** 627,634 ****
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
  
!       PG_RETURN_INT32(arg1 - arg2);
  }
  
  Datum
--- 648,666 ----
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
+       int32           result;
  
!       result = arg1 - arg2;
!       /*
!        * Overflow check.  If the inputs are of the same sign then their
!        * difference cannot overflow.  If they are of different signs then
!        * the result should be of the same sign as the first input.
!        */
!       if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1))
!               ereport(ERROR,
!                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                                errmsg("integer out of range")));
!       PG_RETURN_INT32(result);
  }
  
  Datum
***************
*** 636,643 ****
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
  
!       PG_RETURN_INT32(arg1 * arg2);
  }
  
  Datum
--- 668,695 ----
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
+       int32           result;
  
!       result = arg1 * arg2;
!       /*
!        * Overflow check.  We basically check to see if result / arg2 gives
!        * arg1 again.  There are two cases where this fails: arg2 = 0 (which
!        * cannot overflow) and arg1 = INT_MIN, arg2 = -1 (where the division
!        * itself will overflow and thus incorrectly match).
!        *
!        * Since the division is likely much more expensive than the actual
!        * multiplication, we'd like to skip it where possible.  The best
!        * bang for the buck seems to be to check whether both inputs are in
!        * the int16 range; if so, no overflow is possible.
!        */
!       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)))
!               ereport(ERROR,
!                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                                errmsg("integer out of range")));
!       PG_RETURN_INT32(result);
  }
  
  Datum
***************
*** 645,665 ****
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
  
        if (arg2 == 0)
                ereport(ERROR,
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
  
!       PG_RETURN_INT32(arg1 / arg2);
  }
  
  Datum
  int4inc(PG_FUNCTION_ARGS)
  {
        int32           arg = PG_GETARG_INT32(0);
  
!       PG_RETURN_INT32(arg + 1);
  }
  
  Datum
--- 697,736 ----
  {
        int32           arg1 = PG_GETARG_INT32(0);
        int32           arg2 = PG_GETARG_INT32(1);
+       int32           result;
  
        if (arg2 == 0)
                ereport(ERROR,
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
  
!       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.
!        */
!       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);
  }
  
  Datum
  int4inc(PG_FUNCTION_ARGS)
  {
        int32           arg = PG_GETARG_INT32(0);
+       int32           result;
+ 
+       result = arg + 1;
+       /* Overflow check */
+       if (arg > 0 && result < 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                                errmsg("integer out of range")));
  
!       PG_RETURN_INT32(result);
  }
  
  Datum

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
      subscribe-nomail command to [EMAIL PROTECTED] so that your
      message can get through to the mailing list cleanly

Reply via email to