MIME-Version: 1.0
References: 
 <CAMQXxcgbtD2LXfX0tpgvOizxP-XxrCHV2ZDy4By_TZnJMsxXWQ@mail.gmail.com>
 <FFB7AB27-9334-4F17-BF56-291845F4A9FD@yesql.se>
 <CAMQXxch1N=sZ1fsfx57VhTe=8qpmb1K1vnaB5WVyDbAs8bqExg@mail.gmail.com>
 <1703741.1782745481@sss.pgh.pa.us>
 <CAMQXxch6yrJQdeHQK9Mypa9cmDKjQOc5DJ=SL72pom1djL_OLg@mail.gmail.com>
 <1717564.1782747310@sss.pgh.pa.us>
 <CAMQXxcj=HaLUir91J7K_V7mTwehK0nj62tdhCiH8YwSLtp-A3Q@mail.gmail.com>
 <2231547.1782774522@sss.pgh.pa.us>
In-Reply-To: <2231547.1782774522@sss.pgh.pa.us>
From: Bill Kim <billkimjh@gmail.com>
Date: Wed, 1 Jul 2026 22:38:52 +0900
Message-ID: 
 <CAMQXxcg9e7em0qFzwOg0cQsFVhzRvgAE6DeY2D3kNDX3Ge228g@mail.gmail.com>
Subject: Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Daniel Gustafsson <daniel@yesql.se>, pgsql-bugs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000aac23a06558cd000"
Archived-At: 
 <https://www.postgresql.org/message-id/CAMQXxcg9e7em0qFzwOg0cQsFVhzRvgAE6DeY2D3kNDX3Ge228g%40mail.gmail.com>
Precedence: bulk

--000000000000aac23a06558cd000
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Thanks.
v2 applies cleanly on master and contrib/btree_gist regression passes here.
Nothing to add.

regards,
Bill

2026=EB=85=84 6=EC=9B=94 30=EC=9D=BC (=ED=99=94) =EC=98=A4=EC=A0=84 8:08, T=
om Lane <tgl@sss.pgh.pa.us>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:

> I studied this and decided that it only partially fixes the problem.
> Yes, we've got to fix the comparison functions, but we also have to
> fix the penalty and distance functions.  As things stand, the penalty
> functions will probably return NaN for any input including a NaN,
> which gistpenalty will clamp to zero, which we do not want because
> it'll basically break all decisions about where to put things,
> leading to a seriously inefficient index.  (Maybe gistpenalty should
> do something else with a NaN, but I'm hesitant to touch that right
> now.)  The penalty logic is also under the misapprehension that
> multiplying everything by 0.49 keeps it from having to worry about
> overflows; that won't fix things for infinities.  The distance
> functions will do the wrong things for NaN as well, probably breaking
> any KNN search that happens across a NaN index entry.
>
> So I fixed all that, and then was dismayed to find that the new
> penalty logic wasn't reached at all in the regression tests, per code
> coverage testing.  We don't compute any penalties during GiST index
> build unless it's a multicolumn index, so most of the per-datatype
> tests aren't reaching that.  It didn't seem like adding more index
> entries partway through the test would fit very well into the
> structure of float[48].sql, so instead I added simple tests of
> two-column float indexes to improve the coverage.  Also, I thought
> we should test this logic by adding NaN (and infinities for good
> measure) to the initial data load, rather than creating a new small
> table which would only exercise the logic very minimally.
>
> At the moment I'm leaning to back-patching this.  We'd have to
> release-note the need to reindex any btree_gist indexes containing
> NaNs, but we've done similar things many times before.
>
>                         regards, tom lane
>
>

--000000000000aac23a06558cd000
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Thanks.=C2=A0<div>v2 applies cleanly on master and contrib=
/btree_gist regression passes here.=C2=A0</div><div>Nothing to add.<br><br>=
regards,<br>Bill</div></div><br><div class=3D"gmail_quote gmail_quote_conta=
iner"><div dir=3D"ltr" class=3D"gmail_attr">2026=EB=85=84 6=EC=9B=94 30=EC=
=9D=BC (=ED=99=94) =EC=98=A4=EC=A0=84 8:08, Tom Lane &lt;<a href=3D"mailto:=
tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>&gt;=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=
=84=B1:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px =
0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I studie=
d this and decided that it only partially fixes the problem.<br>
Yes, we&#39;ve got to fix the comparison functions, but we also have to<br>
fix the penalty and distance functions.=C2=A0 As things stand, the penalty<=
br>
functions will probably return NaN for any input including a NaN,<br>
which gistpenalty will clamp to zero, which we do not want because<br>
it&#39;ll basically break all decisions about where to put things,<br>
leading to a seriously inefficient index.=C2=A0 (Maybe gistpenalty should<b=
r>
do something else with a NaN, but I&#39;m hesitant to touch that right<br>
now.)=C2=A0 The penalty logic is also under the misapprehension that<br>
multiplying everything by 0.49 keeps it from having to worry about<br>
overflows; that won&#39;t fix things for infinities.=C2=A0 The distance<br>
functions will do the wrong things for NaN as well, probably breaking<br>
any KNN search that happens across a NaN index entry.<br>
<br>
So I fixed all that, and then was dismayed to find that the new<br>
penalty logic wasn&#39;t reached at all in the regression tests, per code<b=
r>
coverage testing.=C2=A0 We don&#39;t compute any penalties during GiST inde=
x<br>
build unless it&#39;s a multicolumn index, so most of the per-datatype<br>
tests aren&#39;t reaching that.=C2=A0 It didn&#39;t seem like adding more i=
ndex<br>
entries partway through the test would fit very well into the<br>
structure of float[48].sql, so instead I added simple tests of<br>
two-column float indexes to improve the coverage.=C2=A0 Also, I thought<br>
we should test this logic by adding NaN (and infinities for good<br>
measure) to the initial data load, rather than creating a new small<br>
table which would only exercise the logic very minimally.<br>
<br>
At the moment I&#39;m leaning to back-patching this.=C2=A0 We&#39;d have to=
<br>
release-note the need to reindex any btree_gist indexes containing<br>
NaNs, but we&#39;ve done similar things many times before.<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 regards, tom lane<br>
<br>
</blockquote></div>

--000000000000aac23a06558cd000--