Put in_range_float4_float8's work in-line.

In commit 8b29e88cd, I'd dithered about whether to make
in_range_float4_float8 be a standalone copy of the float in-range logic
or have it punt to in_range_float8_float8.  I went with the latter, which
saves code space though at the cost of performance and readability.

However, it emerges that this tickles a compiler or hardware bug on
buildfarm member opossum.  Test results from commit 55e0e4581 show
conclusively that widening a float4 NaN to float8 produces Inf, not NaN,
on that machine; which accounts perfectly for the window RANGE test
failures it's been showing.  We can dodge this problem by making
in_range_float4_float8 be an independent function, so that it checks
for NaN inputs before widening them.

Ordinarily I'd not be very excited about working around such obviously
broken functionality; but given that this was a judgment call to begin
with, I don't mind reversing it.
This commit is contained in:
Tom Lane 2018-05-05 13:21:50 -04:00
parent 2f52518773
commit cb3e9e40bc
1 changed files with 57 additions and 9 deletions

View File

@ -1254,16 +1254,64 @@ in_range_float8_float8(PG_FUNCTION_ARGS)
Datum
in_range_float4_float8(PG_FUNCTION_ARGS)
{
/* Doesn't seem worth duplicating code for, so just invoke float8_float8 */
float8 val = (float8) PG_GETARG_FLOAT4(0);
float8 base = (float8) PG_GETARG_FLOAT4(1);
float4 val = PG_GETARG_FLOAT4(0);
float4 base = PG_GETARG_FLOAT4(1);
float8 offset = PG_GETARG_FLOAT8(2);
bool sub = PG_GETARG_BOOL(3);
bool less = PG_GETARG_BOOL(4);
float8 sum;
return DirectFunctionCall5(in_range_float8_float8,
Float8GetDatumFast(val),
Float8GetDatumFast(base),
PG_GETARG_DATUM(2),
PG_GETARG_DATUM(3),
PG_GETARG_DATUM(4));
/*
* Reject negative or NaN offset. Negative is per spec, and NaN is
* because appropriate semantics for that seem non-obvious.
*/
if (isnan(offset) || offset < 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PRECEDING_FOLLOWING_SIZE),
errmsg("invalid preceding or following size in window function")));
/*
* Deal with cases where val and/or base is NaN, following the rule that
* NaN sorts after non-NaN (cf float8_cmp_internal). The offset cannot
* affect the conclusion.
*/
if (isnan(val))
{
if (isnan(base))
PG_RETURN_BOOL(true); /* NAN = NAN */
else
PG_RETURN_BOOL(!less); /* NAN > non-NAN */
}
else if (isnan(base))
{
PG_RETURN_BOOL(less); /* non-NAN < NAN */
}
/*
* Deal with infinite offset (necessarily +inf, at this point). We must
* special-case this because if base happens to be -inf, their sum would
* be NaN, which is an overflow-ish condition we should avoid.
*/
if (isinf(offset))
{
PG_RETURN_BOOL(sub ? !less : less);
}
/*
* Otherwise it should be safe to compute base +/- offset. We trust the
* FPU to cope if base is +/-inf or the true sum would overflow, and
* produce a suitably signed infinity, which will compare properly against
* val whether or not that's infinity.
*/
if (sub)
sum = base - offset;
else
sum = base + offset;
if (less)
PG_RETURN_BOOL(val <= sum);
else
PG_RETURN_BOOL(val >= sum);
}