Re: BUG #15519: Casting float4 into int4 gets the wrong sign instead of "integer out of range" error
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: victor@magic.io, pgsql-bugs@lists.postgresql.org
Date: 2018-11-23T23:03:47Z
Lists: pgsql-bugs
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "PG" == PG Bug reporting form <noreply@postgresql.org> writes:
> PG> Offending examples:
> PG> SELECT ((2147483647::float4) - 1.0::float4)::int4;
> PG> SELECT ((2147483590::float4) - 1.0::float4)::int4;
> PG> SELECT ((2147483647::float4) + 1.0::float4)::int4;
> PG> They all produce the same result: -2147483648
> The bug here is that the ftoi4 conversion function thinks it can do
> this:
> if (num < INT_MIN || num > INT_MAX || isnan(num))
> Probably this should be changed to use ((float8) INT_MAX) etc. instead?
After thinking about this for awhile, there's another class of undesirable
behavior here, which can be illustrated by
regression=# select '32766.1'::float4::int2;
int2
-------
32766
(1 row)
regression=# select '32767.1'::float4;
float4
---------
32767.1
(1 row)
regression=# select '32767.1'::float4::int2;
ERROR: smallint out of range
It doesn't seem very nice that that fails rather than producing 32767.
I think we could fix that by doing the rint() first, before the
comparisons, which should be safe enough: as the Linux man page for it
points out, it really can't ever fail.
I'm inclined to write ftoi4 as
/*
* Get rid of any fractional part in the input. This is so we don't
* fail on just-out-of-range values that would round into range.
*/
num = rint(num);
/*
* Range check. We must be careful here that the boundary values are
* expressed exactly in the appropriate float domain; we assume float4
* is going to round off INT_MAX to a power of 2. Also note assumption
* that rint() will pass through a NaN or Inf unchanged.
*/
if (unlikely(num < (float4) INT_MIN || num >= (float4) INT_MAX || isnan(num)))
ereport(...);
PG_RETURN_INT32((int32) num);
We could use the same pattern for the other five functions at issue,
but "num >=" would be "num >" in cases where we expect the comparison
value to be represented without roundoff.
Obviously, some regression test cases to verify all these assumptions
would be a good idea.
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