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

ftoi4 and its sibling coercion functions did their overflow checks in
a way that looked superficially plausible, but actually depended on an
assumption that the MIN and MAX comparison constants can be represented
exactly in the float4 or float8 domain.  That fails in ftoi4, ftoi8,
and dtoi8, resulting in a possibility that values near the MAX limit will
be wrongly converted (to negative values) when they need to be rejected.

Also, because we compared before rounding off the fractional part,
the other three functions threw errors for values that really ought
to get rounded to the min or max integer value.

Fix by doing rint() first (requiring an assumption that it handles
NaN and Inf correctly; but dtoi8 and ftoi8 were assuming that already),
and by comparing to values that should coerce to float exactly, namely
INTxx_MIN and -INTxx_MIN.  Also remove some random cosmetic discrepancies
between these six functions.

Per bug #15519 from Victor Petrovykh.  This should get back-patched,
but first let's see what the buildfarm thinks of it --- I'm not too
sure about portability of some of the regression test cases.

Patch by me; thanks to Andrew Gierth for analysis and discussion.

Discussion: https://postgr.es/m/15519-4fc785b483201ff1@postgresql.org

1 files changed, 68 insertions, 11 deletions

diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index c91bb1a3059..cf9327f8853 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -1112,16 +1112,28 @@ Datum
 dtoi4(PG_FUNCTION_ARGS)
 {
 	float8		num = PG_GETARG_FLOAT8(0);
-	int32		result;
 
-	/* 'Inf' is handled by INT_MAX */
-	if (num < INT_MIN || num > INT_MAX || isnan(num))
+	/*
+	 * 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.  Note
+	 * assumption that rint() will pass through a NaN or Inf unchanged.
+	 */
+	num = rint(num);
+
+	/*
+	 * Range check.  We must be careful here that the boundary values are
+	 * expressed exactly in the float domain.  We expect PG_INT32_MIN to be an
+	 * exact power of 2, so it will be represented exactly; but PG_INT32_MAX
+	 * isn't, and might get rounded off, so avoid using it.
+	 */
+	if (unlikely(num < (float8) PG_INT32_MIN ||
+				 num >= -((float8) PG_INT32_MIN) ||
+				 isnan(num)))
 		ereport(ERROR,
 				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
 				 errmsg("integer out of range")));
 
-	result = (int32) rint(num);
-	PG_RETURN_INT32(result);
+	PG_RETURN_INT32((int32) num);
 }
 
 
@@ -1133,12 +1145,27 @@ dtoi2(PG_FUNCTION_ARGS)
 {
 	float8		num = PG_GETARG_FLOAT8(0);
 
-	if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
+	/*
+	 * 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.  Note
+	 * assumption that rint() will pass through a NaN or Inf unchanged.
+	 */
+	num = rint(num);
+
+	/*
+	 * Range check.  We must be careful here that the boundary values are
+	 * expressed exactly in the float domain.  We expect PG_INT16_MIN to be an
+	 * exact power of 2, so it will be represented exactly; but PG_INT16_MAX
+	 * isn't, and might get rounded off, so avoid using it.
+	 */
+	if (unlikely(num < (float8) PG_INT16_MIN ||
+				 num >= -((float8) PG_INT16_MIN) ||
+				 isnan(num)))
 		ereport(ERROR,
 				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
 				 errmsg("smallint out of range")));
 
-	PG_RETURN_INT16((int16) rint(num));
+	PG_RETURN_INT16((int16) num);
 }
 
 
@@ -1174,12 +1201,27 @@ ftoi4(PG_FUNCTION_ARGS)
 {
 	float4		num = PG_GETARG_FLOAT4(0);
 
-	if (num < INT_MIN || num > INT_MAX || isnan(num))
+	/*
+	 * 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.  Note
+	 * assumption that rint() will pass through a NaN or Inf unchanged.
+	 */
+	num = rint(num);
+
+	/*
+	 * Range check.  We must be careful here that the boundary values are
+	 * expressed exactly in the float domain.  We expect PG_INT32_MIN to be an
+	 * exact power of 2, so it will be represented exactly; but PG_INT32_MAX
+	 * isn't, and might get rounded off, so avoid using it.
+	 */
+	if (unlikely(num < (float4) PG_INT32_MIN ||
+				 num >= -((float4) PG_INT32_MIN) ||
+				 isnan(num)))
 		ereport(ERROR,
 				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
 				 errmsg("integer out of range")));
 
-	PG_RETURN_INT32((int32) rint(num));
+	PG_RETURN_INT32((int32) num);
 }
 
 
@@ -1191,12 +1233,27 @@ ftoi2(PG_FUNCTION_ARGS)
 {
 	float4		num = PG_GETARG_FLOAT4(0);
 
-	if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
+	/*
+	 * 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.  Note
+	 * assumption that rint() will pass through a NaN or Inf unchanged.
+	 */
+	num = rint(num);
+
+	/*
+	 * Range check.  We must be careful here that the boundary values are
+	 * expressed exactly in the float domain.  We expect PG_INT16_MIN to be an
+	 * exact power of 2, so it will be represented exactly; but PG_INT16_MAX
+	 * isn't, and might get rounded off, so avoid using it.
+	 */
+	if (unlikely(num < (float4) PG_INT16_MIN ||
+				 num >= -((float4) PG_INT16_MIN) ||
+				 isnan(num)))
 		ereport(ERROR,
 				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
 				 errmsg("smallint out of range")));
 
-	PG_RETURN_INT16((int16) rint(num));
+	PG_RETURN_INT16((int16) num);
 }