Re: Strange behavior with polygon and NaN - Mailing list pgsql-hackers

(My mailer seems to have recovered from unresponsiveness.)

At Tue, 24 Nov 2020 12:29:41 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > At Fri, 20 Nov 2020 15:57:46 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> >> I don't much like anything about float8_coef_mul().
> 
> > I have the same feeling on the function, but I concluded that
> > coefficients and coordinates should be regarded as different things in
> > the practical standpoint.
> 
> > For example, consider Ax + By + C == 0, if B is 0.0, we can remove the
> > second term from the equation, regardless of the value of y, of course
> > even if it were inf. that is, The function imitates that kind of
> > removals.
> 
> Meh --- I can see where you're going with that, but I don't much like it.
> I fear that it's as likely to introduce weird behaviors as remove any.
>
> The core of the issue in
> 
> > | postgres=# select point(1e+300, 'Infinity') <-> line('{1,0,5}');
> > |  ?column? 
> > | ----------
> > |       NaN
> 
> is that we generate the line y = Inf:
> 
> (gdb) p tmp
> $1 = {A = 0, B = -1, C = inf}
> 
> and then try to find the intersection with {1,0,5} (x = -5), but that
> calculation involves 0 * Inf so we get NaNs.  It seems reasonable that
> the intersection should be (-5,Inf), but I don't think we should try
> to force the normal calculation to produce that.  I think we'd be
> better off to explicitly special-case vertical and/or horizontal lines
> in line_interpt_line.

I don't object to have explicit special case for vertical lines since
it is clear than embedding such a function in the formula, but it
seems equivalent to what the function is doing, that is, treating inf
* 0.0 as 0.0 in some special cases.

# And after rethinking, the FPzero() used in the function is wrong
# since the macro (function) is expected to be applied to coordinates,
# not to coefficients.

> Actually though ... even if we successfully got that intersection
> point, we'd still end up with a NaN distance between (1e300,Inf) and
> (-5,Inf), on account of Inf - Inf being NaN.  I think this is correct
> and we'd be ill-advised to try to force it to be something else.
> Although we pretend that two Infs are equal for purposes such as
> sorting, they aren't really, so we should not assume that their
> difference is zero.

The definition "inf == inf" comes from some practical reasons
uncertain to me, and actually inf - inf yields NaN in IEEE
754. However, aren't we going to assume a line on which B is exactly
0.0 as a completely vertical line?  Thus things are slightiy different
from the IEEE's definition.  The "Inf" as the y-coord of the
perpendicular foot is actually "the same y-coord with the point".  So
what we should do on our definition for the calculation is:

perp-foot (line {1,0,5}, point(1e300, Inf)) => point(-5, <y of the point>)
distance (point(1e300, Inf), point(-5, <y of the point>)) => 1e300 (+5)

This is what the code below is doing:

+    return float8_div(fabs(float8_pl(
+                               float8_pl(
+                                   float8_coef_mul(line->A, point->x, false),
+                                   float8_coef_mul(line->B, point->y, false)),
+                               line->C)),
+                      HYPOT(line->A, line->B));

> So that line of thought prompts me to tread *very* carefully when
> trying to dodge NaN results.  We need to be certain that we
> introduce only logically-defensible special cases.  Something like
> float8_coef_mul() seems much more likely to lead us into errors
> than away from them.

Agreed on that point.  I'm going to rewirte the patch in that
direction.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center