Re: Strange behavior with polygon and NaN

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: gkokolatos@pm.me, pgsql-hackers@lists.postgresql.org
Date: 2020-11-20T22:26:46Z
Lists: pgsql-hackers

Attachments

Further to this ...

I realized after looking at things some more that one of
line_closept_point's issues is really a bug in line_construct:
it fails to draw a horizontal line through a point with x = Inf,
though surely that's not particularly ill-defined.  The reason
is that somebody thought they could dispense with a special case
for m == 0, but then we end up doing

	result->C = float8_mi(pt->y, float8_mul(m, pt->x));

and if m = 0 and pt->x = Inf, we get NaN.

It also annoyed me that the code was still using DBL_MAX instead of a
true Inf to represent infinite slope.  That's sort of okay as long as
it's just a communication mechanism between line_construct and places
like line_sl, but it's not really okay, because in some places you can
get a true infinity from a slope calculation.  Thus in HEAD you get
different results from

regression=# select line(point(1,2),point(1,'inf'));
   line   
----------
 {-1,0,1}
(1 row)

regression=# select line(point(1,2),point(4,'inf'));
          line           
-------------------------
 {Infinity,-1,-Infinity}
(1 row)

which is completely silly: we ought to "round off" that infinitesimal
slope to a true vertical, rather than producing a line representation
we can't do anything with.

So I fixed that too, but then I got a weird regression test diff:
the case of
	lseg '[(-10,2),(-10,3)]' ?|| lseg '[(-10,2),(-10,3)]'
was no longer returning true.  The reason turned out to be that
lseg_parallel does

	PG_RETURN_BOOL(FPeq(lseg_sl(l1), lseg_sl(l2)));

and now lseg_sl is returning true infinities for vertical lines, and
FPeq *gets the wrong answer* when asked to compare Inf to Inf.  It
should say equal, surely, but internally it computes a NaN and ends up
with false.

So the attached 0003 patch also fixes FPeq() and friends to give
sane answers for Inf-vs-Inf comparisons.  That part seems like
a fairly fundamental bug fix, and so I feel like we ought to
go ahead and apply it before we do too much more messing with
the logic in this area.

(Note that the apparently-large results diff in 0003 is mostly
a whitespace change: the first hunk just reflects slopes coming
out as Infinity not DBL_MAX.)

I'm reposting 0001 and 0002 just to keep the cfbot happy,
they're the same as in my previous message.

			regards, tom lane

Commits

  1. In geo_ops.c, represent infinite slope as Infinity, not DBL_MAX.

  2. Fix FPeq() and friends to get the right answers for infinities.

  3. Extend the geometric regression test cases a little.

  4. Remove underflow error in float division with infinite divisor.