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
-
Update additional float4/8 expected-output files.
- bf9fb00dd107 9.4.21 landed
- 74587148314a 10.7 landed
- 3645d31934f4 11.2 landed
- 298510caee63 9.5.16 landed
- 1f99d0867020 9.6.12 landed
-
Fix float-to-integer coercions to handle edge cases correctly.
- d5231253e309 9.4.21 landed
- 93eec1238663 9.6.12 landed
- 1e78603a54a7 9.5.16 landed
- e473e1f2b8f1 11.2 landed
- c382a2b66909 10.7 landed
- cbdb8b4c0155 12.0 landed
-
Adjust new test case for more portability.
- 452b637d4b02 12.0 landed