public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bill Kim <[email protected]>
To: Tom Lane <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses
Date: Wed, 1 Jul 2026 22:38:52 +0900
Message-ID: <CAMQXxcg9e7em0qFzwOg0cQsFVhzRvgAE6DeY2D3kNDX3Ge228g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAMQXxcgbtD2LXfX0tpgvOizxP-XxrCHV2ZDy4By_TZnJMsxXWQ@mail.gmail.com>
<[email protected]>
<CAMQXxch1N=sZ1fsfx57VhTe=8qpmb1K1vnaB5WVyDbAs8bqExg@mail.gmail.com>
<[email protected]>
<CAMQXxch6yrJQdeHQK9Mypa9cmDKjQOc5DJ=SL72pom1djL_OLg@mail.gmail.com>
<[email protected]>
<CAMQXxcj=HaLUir91J7K_V7mTwehK0nj62tdhCiH8YwSLtp-A3Q@mail.gmail.com>
<[email protected]>
Thanks.
v2 applies cleanly on master and contrib/btree_gist regression passes here.
Nothing to add.
regards,
Bill
2026년 6월 30일 (화) 오전 8:08, Tom Lane <[email protected]>님이 작성:
> 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
>
>
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19524: NaN handling in btree_gist's float4/float8 opclasses
In-Reply-To: <CAMQXxcg9e7em0qFzwOg0cQsFVhzRvgAE6DeY2D3kNDX3Ge228g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox