Re: BUG #15519: Casting float4 into int4 gets the wrong sign instead of "integer out of range" error

Victor Petrovykh <victor@magic.io>

From: Victor Petrovykh <victor@magic.io>
To: tgl@sss.pgh.pa.us
Cc: andrew@tao11.riddles.org.uk, pgsql-bugs@lists.postgresql.org
Date: 2018-11-28T18:14:17Z
Lists: pgsql-bugs
Thanks for the clarification.

I was thinking about the assumption whether or not INT_MAX is representable
perfectly as a float. Consider any combination of types int_X and float_Y
where X and Y denote the number of bits used for representation:
- for any type int_X it is always true that its maximum value cannot be
represented by a float_Y if X >= Y because at least one bit of the float_Y
must be used to represent the exponent part of the float (else the float_Y
is indistinguishable from int_Y)
- for any type int_X it is always possible to represent the maximum value
(and by extension any other value) exactly as a float_Y if X <= M(Y),
meaning that if the mantissa of the float_Y has at least as many bits as X.
In practice X and Y will be some form of 8, 16, 32, 64, etc. so 2X <= Y and
unless the mantissa is less than half of the significant bits of the float
we will have x <= M(Y) for any practical X < Y
- for any int_X and float_Y there exists a specific value FLOATY, such that
for all possible float_Y values y < FLOATY it is also true that y <=
MAX_INT_X and for all y >= FLOATY it is also true that y > MAX_INT
(basically I can always pick a float threshold above which all numbers
would be above MAX_INT and therefore out of range and below it would be
guaranteed to be representable as ints). I conjecture that this special
value FLOATY = (float_Y)MAX_INT_X for any X >= Y. I think I can formally
prove this conjecture, basically it has to do with sparseness of float
values when compared to MAX_INT_X + 1 and MAX_INT_X - 1.

So it seems to me that the rule for casting would depend on whether the
float has same or fewer bits than the int or not:
- when the int has same or more bits (e.g. float4 -> int4 or float4 -> int8)

  if (num < INT_MIN || num >= INT_MAX || isnan(num))

- when the int has fewer bits (e.g. float4 -> int2 or float8 -> int4)

  if (num < INT_MIN || num > INT_MAX || isnan(num))

I don't think I make any unsafe architecture assumptions in the above. As
far as I can tell the only such assumption is about likely values of X and
Y being 8, 16, 32, 64, 128, etc. and that the mantissa of a float is using
more than half of its bits. I'm assuming that casting function knows the 2
actual types so it can make a deterministic decision about the comparison
it must use.

I have one more related question: is a fix for this likely to appear in the
next Postgres release?


On Fri, Nov 23, 2018 at 8:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Victor Petrovykh <victor@magic.io> writes:
> > Am I missing something in thinking that the cast of a float to int should
> > produce an error if the sign of the original value doesn't match the sign
> > of the cast result?
>
> Well, yeah, our traditional way of coding this overflow check would
> probably have compared the sign of the result to the sign of the input,
> but (a) we'd still have needed some ad-hoc range check to avoid getting
> fooled by inputs large enough to have wrapped around an even number of
> times, and (b) this approach depends on the compiler not taking any
> liberties based on an assumption that the program doesn't provoke
> integer overflow.  (b) gets more worrisome with each year that goes by,
> because the compiler guys keep finding ever-more-creative ways to break
> your code if it violates C-spec semantics.  So we really want to write
> a test that will fail exactly when the integer coercion would overflow,
> not do the coercion and then check to see if it overflowed.
>
>                         regards, tom lane
>

Commits

  1. Update additional float4/8 expected-output files.

  2. Fix float-to-integer coercions to handle edge cases correctly.

  3. Adjust new test case for more portability.