Re: Issue with point_ops and NaN

At Wed, 31 Mar 2021 12:04:26 +0800, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote in
> On Tue, Mar 30, 2021 at 11:39:40PM +0800, Julien Rouhaud wrote:
> > On Tue, Mar 30, 2021 at 11:02:32AM -0400, Tom Lane wrote:
> > > Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> > > > On Tue, Mar 30, 2021 at 02:47:05PM +0200, Laurenz Albe wrote:
> > > >> I'd say that this is certainly wrong:
> > > >> SELECT point('NaN','NaN') <@ polygon('(0,0),(1,0),(1,1),(0,0)');
> > > >>
> > > >> ?column?
> > > >> ----------
> > > >> t
> > > >> (1 row)
> > >
> > > > Yeah that's what I think too, but I wanted to have confirmation.
> > >
> > > Agreed --- one could make an argument for either 'false' or NULL
> > > result, but surely not 'true'.
> >
> > I would think that it should return NULL since it's not inside nor outside the
> > polygon, but I'm fine with false.
> >
> > > I wonder if Horiguchi-san's patch [1] improves this case.
> >
> > Oh I totally missed that patch :(
> >
> > After a quick look I see this addition in point_inside():
> >
> > + /* NaN makes the point cannot be inside the polygon */
> > + if (unlikely(isnan(x) || isnan(y)))
> > + return 0;
> >
> > So I would assume that it should fix this case too. I'll check tomorrow.
>
> I confirm that this patch fixes the issue, and after looking a bit more at the
> thread it's unsurprising since Jesse initially reported the exact same problem.
>
> I'll try to review it as soon as I'll be done with my work duties.

Thanks! However, Michael's suggestion is worth considering. What do
you think about makeing NaN-involved comparison return NULL? If you
agree to that, I'll make a further change to the patch.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center