Michaël,

attached is a first cut that I would like to commit separately which adds all the compatibility overflow routines to int.h for uint16, uint32 and uint64 with all the fallback implementations (int128-based method added as well if available). I have also grouped at the top of the file the comments about each routine's return policy to avoid duplication. For the fallback implementations of uint64 using int128, I think that it is cleaner to use uint128 so as there is no need to check after negative results for multiplications, and I have made the various expressions consistent for each size.

Patch applies cleanly, compiles, "make check" ok, but the added functions are not used (yet).

I think that factoring out comments is a good move.

For symmetry and efficiency, ISTM that uint16 mul overflow could use uint32 and uint32 could use uint64, and the division-based method be dropped in these cases.

Maybe I would add a comment before each new section telling about the type, eg:

 /*
  * UINT16
  */
 add/sub/mul uint16 functions…

I would tend to commit working solutions per type rather than by installment, so that at least all added functions are actually used somewhere, but it does not matter much, really.

I was unsure that having int128 implies uint128 availability, so I did not use it.

The checking extension is funny, but ISTM that these checks should be (are already?) included in some standard sql test, which will test the macros from direct SQL operations:

  fabien=# SELECT INT2 '1234512434343';
  ERROR:  value "1234512434343" is out of range for type smallint

Well, a quick look at "src/test/regress/sql/int2.sql" suggests that
there the existing tests should be extended… :-(

--
Fabien.

Reply via email to