Re: Numeric x^y for negative x - Mailing list pgsql-hackers

On Wed, 7 Jul 2021 18:36:56 +0100
Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

> On Thu, 1 Jul 2021 at 14:17, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> >
> > On Tue, 29 Jun 2021 at 12:08, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > >
> > > Numeric x^y is supported for x < 0 if y is an integer, but this
> > > currently fails if y is outside the range of an int32
> >
> > I've been doing some more testing of this, and I spotted another
> > problem with numeric_power().
> >
> > [loss of precision and overflow errors]
> >
> > I think we should attempt to avoid all such overflow errors,
> > that are actually underflows, and return zero instead.
> >
> 
> Finally getting back to this ... attached is an updated patch that now
> includes a fix for the loss-of-precision bug and the overflow errors.
> I don't think it's really worth trying to split these up, since
> they're all somewhat interrelated.

The patch can be applied cleanly.
(Though, I need to remove lines "new file mode 100644" else I get an error
 "error: git apply: bad git-diff - expected /dev/null on line 4".)

Compilation succeeded, and all tests passed.

This patch fixes numeric_power() to handle negative bases correctly and not
to raise an error "cannot take logarithm of a negative number", as well as a
bug that a result whose values is almost zero is incorrectly returend as 1.
The previous behaviors are obvious strange, and these fixes seem to me reasonable.

Also, improper overflow errors are corrected in numeric_power() and
numeric_exp() to return 0 when it is underflow in fact.
I think it is no problem that these functions return zero instead of underflow
errors because power_var_int() already do the same.

The patch includes additional tests for checking negative bases cases and
underflow and rounding of the almost zero results. It seems good.

Let me just make one comment.

                (errcode(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION),
                 errmsg("zero raised to a negative power is undefined")));

-   if (sign1 < 0 && !numeric_is_integral(num2))
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION),
-                errmsg("a negative number raised to a non-integer power yields a complex result")));
-
    /*
     * Initialize things
     */

I don't think we need to move this check from numeric_power to power_var.
I noticed the following comment in a numeric_power(). 

    /*    
     * The SQL spec requires that we emit a particular SQLSTATE error code for
     * certain error conditions.  Specifically, we don't return a
     * divide-by-zero error code for 0 ^ -1.
     */

In the original code, two checks that could raise an error of
ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION are following the comment.
I think these check codes are placed together under this comment intentionally,
so I suggest not to move one of them.


Regards,
Yugo Nagata

-- 
Yugo NAGATA <nagata@sraoss.co.jp>