Thread: Re: power() function in Windows: "value out of range: underflow"
David Rowley <david.rowley@2ndquadrant.com> writes:
> You patch fixes the problem for me, and importantly the two following
> cases still work:
> postgres=# select power(1,'NaN');
> power
> -------
> 1
> (1 row)
> postgres=# select power('NaN', 0);
> power
> -------
> 1
> (1 row)
> There's no mention in the SQL standard about NaN handling in the above
> two cases, but I wonder if it's worth also adding a couple of tests
> for the above two cases to ensure all platforms are doing the same
> thing... ?
So the early returns from the buildfarm say "no, they aren't". It looks
like older BSDen know the NaN^0 = 1 rule, but they return NaN for 1^NaN.
Aside from the failure buildfarm member nightjar is showing, I've
confirmed this behavior on a NetBSD 5.2 installation I had laying around,
and there's also evidence in the related man page:
http://netbsd.gw.com/cgi-bin/man-cgi?pow+3+NetBSD-5.2.3
which goes to great lengths to justify NaN^0 = 1 while saying nothing
to suggest that 1^NaN might not yield NaN.
I'm not sure if we should add more special-case code for that, or just
remove that test case again. Historically we've not really felt that it
was our job to mask oddities of the platform's math library, so the fact
that power() is worrying about this seems like unjustified scope creep.
On the other hand, we do have a bunch of special cases there already,
so refusing to handle older BSD would be a tad inconsistent.
And, while we're on the subject ... some of my machines give
postgres=# select power(-1,'NaN');
ERROR: a negative number raised to a non-integer power yields a complex result
POSIX says this should return NaN, not an error (and my old NetBSD
installation does that, as does numeric_power). Should we change it?
I'm a bit inclined to handle all the NaN-input cases with explicit
code before we do anything that relies on libc's behavior.
BTW, numeric_power just emits NaN for NaN input, without either of
these special cases. But that's platform-independent, so I'm hesitant
to consider back-patching a change there; if we change that, I'd vote
for doing so only in HEAD.
regards, tom lane
On 30 April 2018 at 09:19, Tom Lane <tgl@sss.pgh.pa.us> wrote: > http://netbsd.gw.com/cgi-bin/man-cgi?pow+3+NetBSD-5.2.3 > > which goes to great lengths to justify NaN^0 = 1 while saying nothing > to suggest that 1^NaN might not yield NaN. > > I'm not sure if we should add more special-case code for that, or just > remove that test case again. Historically we've not really felt that it > was our job to mask oddities of the platform's math library, so the fact > that power() is worrying about this seems like unjustified scope creep. > On the other hand, we do have a bunch of special cases there already, > so refusing to handle older BSD would be a tad inconsistent. (Sorry missed your reply before I sent my last one) Wouldn't this machine have returned 1 before this patch though? Surely changing this behaviour this plaetform in favour of fixing on another is worse than doing nothing. If that's the case, I think the only option is to add a special case or revert this and document that the behaviour may vary depending on the platform's implementation of pow(). I think the special case is worth it, since there's already some for the error cases. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
David Rowley <david.rowley@2ndquadrant.com> writes:
> Wouldn't this machine have returned 1 before this patch though?
No, don't think so, because it doesn't set EDOM for the case.
Basically what we're doing here is making sure that we get results
conforming to current POSIX even on machines that predate that
standard. There are more of them floating around than I'd have
expected, but it still seems like a good change to make. Maybe
there's an argument for not back-patching, though?
regards, tom lane
On 30 April 2018 at 10:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> Wouldn't this machine have returned 1 before this patch though?
>
> No, don't think so, because it doesn't set EDOM for the case.
>
> Basically what we're doing here is making sure that we get results
> conforming to current POSIX even on machines that predate that
> standard. There are more of them floating around than I'd have
> expected, but it still seems like a good change to make. Maybe
> there's an argument for not back-patching, though?
I think we should back patch and try to be consistent about the
power(float8 1.0, 'NaN') and power('NaN', float8 0.0) cases. The
archives don't show any complaints about power() with NaN until this
one, so I imagine the number of people affected by this is small.
However, I think if we're willing to try to make MSVC consistent with
other platforms on this topic then there's no reason to draw the line
there and ignore other platforms that we claim to support.
POSIX seems like a good standard to follow for this in the absence of
guidance from the SQL standard.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Sun, Apr 29, 2018 at 7:24 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> I think we should back patch and try to be consistent about the
> power(float8 1.0, 'NaN') and power('NaN', float8 0.0) cases. The
> archives don't show any complaints about power() with NaN until this
> one, so I imagine the number of people affected by this is small.
I agree that this is not likely to affect a lot of people -- but the
question isn't how many people will be affected but rather, of those
that are, how many of them will be pleased rather than displeased by a
change. I would argue that the results have to be unambiguously wrong
in the back-branches to justify a change there, and this doesn't
appear to meet that standard. I would guess that the number of people
who use NaN is very small, but those people have probably adapted
their application to the behavior they are getting currently.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Apr 29, 2018 at 7:24 PM, David Rowley
> <david.rowley@2ndquadrant.com> wrote:
>> I think we should back patch and try to be consistent about the
>> power(float8 1.0, 'NaN') and power('NaN', float8 0.0) cases. The
>> archives don't show any complaints about power() with NaN until this
>> one, so I imagine the number of people affected by this is small.
> I agree that this is not likely to affect a lot of people -- but the
> question isn't how many people will be affected but rather, of those
> that are, how many of them will be pleased rather than displeased by a
> change. I would argue that the results have to be unambiguously wrong
> in the back-branches to justify a change there, and this doesn't
> appear to meet that standard. I would guess that the number of people
> who use NaN is very small, but those people have probably adapted
> their application to the behavior they are getting currently.
The point here, I think, is that you get behavior X on approximately 100%
of modern platforms, but (without this patch) behavior Y on some number of
older platforms. People who have tested their app on a modern platform
and then find that it misbehaves on an old one will think this is a bug
fix. People who only run their app on an old platform may think the
pre-patch behavior is fine, in which case they will indeed be upset if
we change it in a minor release. Are there more of the latter than the
former? I don't really know, and you don't either. But I don't think
we should discount the existence of the former category. Deploying
to production on an older release of $system than you develop on
is hardly an unusual scenario.
regards, tom lane
On Tue, May 1, 2018 at 1:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The point here, I think, is that you get behavior X on approximately 100% > of modern platforms, but (without this patch) behavior Y on some number of > older platforms. People who have tested their app on a modern platform > and then find that it misbehaves on an old one will think this is a bug > fix. People who only run their app on an old platform may think the > pre-patch behavior is fine, in which case they will indeed be upset if > we change it in a minor release. Are there more of the latter than the > former? I don't really know, and you don't either. I agree with all of that. > But I don't think > we should discount the existence of the former category. Deploying > to production on an older release of $system than you develop on > is hardly an unusual scenario. That's probably true, but making dev, test, and production boxes similar is generally good practice and users can do as much or as little of it as they find they need in order to avoid getting burned. They can't do anything about behavior changes we inject into minor releases. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> But I don't think
> we should discount the existence of the former category. Deploying
> to production on an older release of $system than you develop on
> is hardly an unusual scenario.
That's probably true, but making dev, test, and production boxes
similar is generally good practice and users can do as much or as
little of it as they find they need in order to avoid getting burned.
They can't do anything about behavior changes we inject into minor
releases.
+1; this doesn't seem clear-cut and important enough to deviate from the (for me) preferable position of leaving well-enough-alone in the back branches.
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Tue, May 1, 2018 at 12:08 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> That's probably true, but making dev, test, and production boxes
>> similar is generally good practice and users can do as much or as
>> little of it as they find they need in order to avoid getting burned.
>> They can't do anything about behavior changes we inject into minor
>> releases.
> +1; this doesn't seem clear-cut and important enough to deviate from the
> (for me) preferable position of leaving well-enough-alone in the back
> branches.
Well, I seem to be in the minority here, so unless additional people
chime in, I'll revert this in the back branches shortly.
regards, tom lane