Re: Bug in GiST paring heap comparator - Mailing list pgsql-hackers

From Nikita Glukhov
Subject Re: Bug in GiST paring heap comparator
Date
Msg-id e9547b97-ed11-8e0e-6f40-0777d2407207@postgrespro.ru
Whole thread Raw
In response to Re: Bug in GiST paring heap comparator  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: Bug in GiST paring heap comparator
List pgsql-hackers


On 09.09.2019 22:47, Alexander Korotkov wrote:
On Mon, Sep 9, 2019 at 8:32 PM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
On 08.09.2019 22:32, Alexander Korotkov wrote:

On Fri, Sep 6, 2019 at 5:44 PM Alexander Korotkov
<a.korotkov@postgrespro.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

pgsql-hackers by date:

Previous
From: Alvaro Herrera from 2ndQuadrant
Date:
Subject: Re: Libpq support to connect to standby server as priority
Next
From: "Tsunakawa, Takayuki"
Date:
Subject: RE: SIGQUIT on archiver child processes maybe not such a hot idea?