From: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
---|---|
To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Subject: | Re: Bug in GiST paring heap comparator |
Date: | 2019-09-11 00:31:34 |
Message-ID: | e9547b97-ed11-8e0e-6f40-0777d2407207@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 09.09.2019 22:47, Alexander Korotkov wrote:
> On Mon, Sep 9, 2019 at 8:32 PM Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>> On 08.09.2019 22:32, Alexander Korotkov wrote:
>>
>>> On Fri, Sep 6, 2019 at 5:44 PM Alexander Korotkov
>>> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
>>>> I'm going to push both if no objections.
>>> So, pushed!
>> Two years ago there was a similar patch for this issue:
>> https://www.postgresql.org/message-id/1499c9d0-075a-3014-d2aa-ba59121b3728%40postgrespro.ru
>>
>> Sorry that I looked at this thread too late.
> I see, thanks.
>
> You patch seems a bit less cumbersome thanks to using GISTDistance
> struct. You don't have to introduce separate array with null flags.
> But it's harder too keep index_store_float8_orderby_distances() used
> by both GiST and SP-GiST.
>
> What do you think? You can rebase your patch can propose that as refactoring.
Rebased patch with refactoring is attached.
During this rebase I found that SP-GiST code has similar problems with NULLs.
All SP-GiST functions do not check SK_ISNULL flag of ordering ScanKeys, and
that leads to segfaults. In the following test, index is searched with
non-NULL key first and then with NULL key, that leads to a crash:
CREATE TABLE t AS SELECT point(x, y) p
FROM generate_series(0, 100) x, generate_series(0, 100) y;
CREATE INDEX ON t USING spgist (p);
-- crash
SELECT (SELECT p FROM t ORDER BY p <-> pt LIMIT 1)
FROM (VALUES (point '1,2'), (NULL)) pts(pt);
After adding SK_ISNULL checks and starting to produce NULL distances, we need to
store NULL flags for distance somewhere. Existing singleton flag for the whole
SPGistSearchItem is not sufficient anymore.
So, I introduced structure IndexOrderByDistance and used it everywhere in the
GiST and SP-GiST code instead of raw double distances.
SP-GiST order-by-distance code can be further refactored so that user functions
do not have to worry about SK_ISNULL checks.
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-GiST-and-SP-GiST-ordering-by-distance-for-NULLs.patch | text/x-patch | 30.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tsunakawa, Takayuki | 2019-09-11 01:36:15 | RE: SIGQUIT on archiver child processes maybe not such a hot idea? |
Previous Message | Alvaro Herrera from 2ndQuadrant | 2019-09-11 00:17:31 | Re: Libpq support to connect to standby server as priority |