Subject: Re: Bug in GiST paring heap comparator
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
References: 
 <CAPpHfdsNvNdA0DBS+wMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ@mail.gmail.com>
 <7362fcac-3664-d960-f0fa-77c24e62a7b3@iki.fi>
 <CAPpHfdtaQZLC_9Zyn9rFMoxVKPimDBce0SwKvT5+7XH=z_xYvA@mail.gmail.com>
 <CAPpHfdvgx7hM4wJmeEeaiuYTTZjzsokJyhfL1h7Gt-sxKZidZQ@mail.gmail.com>
 <826f57ee-afc7-8977-c44c-6111d18b02ec@postgrespro.ru>
 <CAPpHfdskuYR5tN78u7Yk_H9SQdnjnYFZZ8aQ69jRbXrRVzV=9g@mail.gmail.com>
 <e9547b97-ed11-8e0e-6f40-0777d2407207@postgrespro.ru>
 <CAPpHfdvttSd3XsVp5dSgBKz=KxakWtBnbgCeJULJDRpVkB_K8A@mail.gmail.com>
 <ebcee467-547e-ca63-1858-4a485c4b8261@postgrespro.ru>
 <CAPpHfdtkvxXtxT7KADq+_Y894qM6ybAuF10rYp1bxfvYfW6dLw@mail.gmail.com>
 <907ff6e7-001c-8e86-e05f-ffe5483c2634@postgrespro.ru>
 <CAPpHfdsnNdryJ765=oqw16Nv4fKdHEJ+T5Tm07asOL6a4s3W9g@mail.gmail.com>
 <CAPpHfdsr-YceL5JyJ4DQ5O8mBksYBaS5S+rbei3Vap6hTZ4KKQ@mail.gmail.com>
 <426580d3-a668-b9d1-7b8e-f74d1a6524e0@postgrespro.ru>
Organization: Postgres Professional
Message-ID: <53536807-784c-e029-6e92-6da802ab8d60@postgrespro.ru>
Date: Wed, 25 Sep 2019 01:19:52 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <426580d3-a668-b9d1-7b8e-f74d1a6524e0@postgrespro.ru>
Content-Type: multipart/mixed;
 boundary="------------CAFD5497F917965B65AE5DD1"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------CAFD5497F917965B65AE5DD1
Content-Type: multipart/alternative;
 boundary="------------D1E3ED95C46FA0576CB2DFC4"


--------------D1E3ED95C46FA0576CB2DFC4
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On 20.09.2019 0:15, Nikita Glukhov wrote:
> On 19.09.2019 22:14, Alexander Korotkov wrote:
>> Pushed.
> Attached patch fixes premature xs_orderbynulls[] assignment.  The old value
> of NULL flag, not the new, should be checked before pfree()ing the old value.

Attached another one-line patch that fixes incorrect number of distances used
in pairingheap_SpGistSearchItem_cmp():

-    for (i = 0; i < so->numberOfOrderBys; i++)
+    for (i = 0; i < so->numberOfNonNullOrderBys; i++)


This change was present in my original commit, but it seems to have been
missed after the rebase.  Sorry.

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--------------D1E3ED95C46FA0576CB2DFC4
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 20.09.2019 0:15, Nikita Glukhov wrote:<br>
    <blockquote type="cite"
      cite="mid:426580d3-a668-b9d1-7b8e-f74d1a6524e0@postgrespro.ru">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">On 19.09.2019 22:14, Alexander
        Korotkov wrote:<br>
      </div>
      <blockquote type="cite"
cite="mid:CAPpHfdsr-YceL5JyJ4DQ5O8mBksYBaS5S+rbei3Vap6hTZ4KKQ@mail.gmail.com">
        <meta http-equiv="content-type" content="text/html;
          charset=UTF-8">
        <div dir="ltr">
          <div class="gmail_quote">
            <div>Pushed.</div>
          </div>
        </div>
      </blockquote>
      <div class="moz-signature"> </div>
      <div class="moz-signature">
        <pre>Attached patch fixes premature xs_orderbynulls[] assignment.  The old value 
of NULL flag, not the new, should be checked before pfree()ing the old value.</pre>
      </div>
    </blockquote>
    <pre>Attached another one-line patch that fixes incorrect number of distances used 
in pairingheap_SpGistSearchItem_cmp():
</pre>
    <pre>
-    for (i = 0; i &lt; so-&gt;numberOfOrderBys; i++)
+    for (i = 0; i &lt; so-&gt;numberOfNonNullOrderBys; i++)


This change was present in my original commit, but it seems to have been 
missed after the rebase.  Sorry.
</pre>
    <p>--<br>
      Nikita Glukhov<br>
      Postgres Professional: <a href="http://www.postgrespro.com">http://www.postgrespro.com</a><br>
      The Russian Postgres Company<br>
    </p>
  </body>
</html>

--------------D1E3ED95C46FA0576CB2DFC4--

--------------CAFD5497F917965B65AE5DD1
Content-Type: text/x-patch;
 name="0001-Fix-number-of-distances-in-pairingheap_SpGistSearchI.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0001-Fix-number-of-distances-in-pairingheap_SpGistSearchI.pa";
 filename*1="tch"

From 2e58a94fd2f973257d1c8a3d9cd0b907c719a859 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Tue, 24 Sep 2019 01:33:03 +0300
Subject: [PATCH] Fix number of distances in pairingheap_SpGistSearchItem_cmp()

---
 src/backend/access/spgist/spgscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c
index cfba470..6e940fd 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -56,7 +56,7 @@ pairingheap_SpGistSearchItem_cmp(const pairingheap_node *a,
 	else
 	{
 		/* Order according to distance comparison */
-		for (i = 0; i < so->numberOfOrderBys; i++)
+		for (i = 0; i < so->numberOfNonNullOrderBys; i++)
 		{
 			if (isnan(sa->distances[i]) && isnan(sb->distances[i]))
 				continue;		/* NaN == NaN */
-- 
2.7.4


--------------CAFD5497F917965B65AE5DD1--