Re: Bug in numeric multiplication - Mailing list pgsql-hackers

I wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> The problem then arises in the final carry propagation pass. During
>> this phase of the computation, the carry from one digit (which can be
>> a shade under INT_MAX / NBASE) is added to the next digit, and that's
>> where the overflow happens.

> Nice catch!  I think the comment could use a little more work, but I'll
> adjust it and push.

After trying to rework the comment to explain what maxdig really meant
after your changes, I came to the conclusion that it'd be better to do
it as per attached.  Does this look sane to you?

            regards, tom lane

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 1bfa29e..d403554 100644
*** a/src/backend/utils/adt/numeric.c
--- b/src/backend/utils/adt/numeric.c
*************** mul_var(NumericVar *var1, NumericVar *va
*** 5789,5796 ****
       * to avoid normalizing carries immediately.
       *
       * maxdig tracks the maximum possible value of any dig[] entry; when this
!      * threatens to exceed INT_MAX, we take the time to propagate carries. To
!      * avoid overflow in maxdig itself, it actually represents the max
       * possible value divided by NBASE-1.
       */
      dig = (int *) palloc0(res_ndigits * sizeof(int));
--- 5789,5801 ----
       * to avoid normalizing carries immediately.
       *
       * maxdig tracks the maximum possible value of any dig[] entry; when this
!      * threatens to exceed INT_MAX, we take the time to propagate carries.
!      * Furthermore, we need to ensure that overflow doesn't occur during the
!      * carry propagation pass below either.  The carry value could be as much
!      * as INT_MAX/NBASE, so really we should normalize when digits threaten to
!      * exceed INT_MAX - INT_MAX/NBASE.
!      *
!      * To avoid overflow in maxdig itself, it actually represents the max
       * possible value divided by NBASE-1.
       */
      dig = (int *) palloc0(res_ndigits * sizeof(int));
*************** mul_var(NumericVar *var1, NumericVar *va
*** 5806,5812 ****

          /* Time to normalize? */
          maxdig += var1digit;
!         if (maxdig > INT_MAX / (NBASE - 1))
          {
              /* Yes, do it */
              carry = 0;
--- 5811,5817 ----

          /* Time to normalize? */
          maxdig += var1digit;
!         if (maxdig > (INT_MAX - INT_MAX / NBASE) / (NBASE - 1))
          {
              /* Yes, do it */
              carry = 0;
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index e6ee548..c1886fd 100644
*** a/src/test/regress/expected/numeric.out
--- b/src/test/regress/expected/numeric.out
*************** SELECT * FROM num_input_test;
*** 1334,1339 ****
--- 1334,1366 ----
  (7 rows)

  --
+ -- Test some corner cases for multiplication
+ --
+ select 4790999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+                                                                                      ?column?
                                                            
+
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
47909999999999999999999999999999999999999999999999999999999999999999999999999999999999985209000000000000000000000000000000000000000000000000000000000000000000000000000000000001
+ (1 row)
+
+ select 4789999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+                                                                                      ?column?
                                                            
+
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
47899999999999999999999999999999999999999999999999999999999999999999999999999999999999985210000000000000000000000000000000000000000000000000000000000000000000000000000000000001
+ (1 row)
+
+ select 4770999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+                                                                                      ?column?
                                                            
+
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
47709999999999999999999999999999999999999999999999999999999999999999999999999999999999985229000000000000000000000000000000000000000000000000000000000000000000000000000000000001
+ (1 row)
+
+ select 4769999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+                                                                                      ?column?
                                                            
+
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
47699999999999999999999999999999999999999999999999999999999999999999999999999999999999985230000000000000000000000000000000000000000000000000000000000000000000000000000000000001
+ (1 row)
+
+ --
  -- Test some corner cases for division
  --
  select 999999999999999999999::numeric/1000000000000000000000;
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 982287c..49ec478 100644
*** a/src/test/regress/sql/numeric.sql
--- b/src/test/regress/sql/numeric.sql
*************** INSERT INTO num_input_test(n1) VALUES ('
*** 822,827 ****
--- 822,839 ----
  SELECT * FROM num_input_test;

  --
+ -- Test some corner cases for multiplication
+ --
+
+ select 4790999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+
+ select 4789999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+
+ select 4770999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+
+ select 4769999999999999999999999999999999999999999999999999999999999999999999999999999999999999 *
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999;
+
+ --
  -- Test some corner cases for division
  --