Message-ID: <668dc67a-9baf-e4ab-88b0-2c39c6a2f08e@enterprisedb.com>
Date: Fri, 5 Nov 2021 02:09:59 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.1.0
Subject: Re: [sqlsmith] Failed assertion in brin_minmax_multi_distance_float4
 on REL_14_STABLE
Content-Language: en-US
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Justin Pryzby <pryzby@telsasoft.com>,
 Andreas Seltenreich <andreas.seltenreich@credativ.de>
Cc: pgsql-hackers@postgresql.org,
 Jaime Casanova <jcasanov@systemguards.com.ec>
References: <87r1bw9ukm.fsf@credativ.de>
 <20211104165357.GU17618@telsasoft.com>
 <83b2e911-ccd9-5503-fb32-369050a6e93d@enterprisedb.com>
In-Reply-To: <83b2e911-ccd9-5503-fb32-369050a6e93d@enterprisedb.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/668dc67a-9baf-e4ab-88b0-2c39c6a2f08e%40enterprisedb.com>
Precedence: bulk


On 11/4/21 23:56, Tomas Vondra wrote:
> Hi,
> 
> On 11/4/21 17:53, Justin Pryzby wrote:
>> On Thu, Nov 04, 2021 at 09:46:49AM +0100, Andreas Seltenreich wrote:
>>> sqlsmith triggers the following assertion when testing REL_14_STABLE:
>>>
>>>      TRAP: FailedAssertion("a1 <= a2", File: "brin_minmax_multi.c", 
>>> Line: 1879, PID: 631814)
>>>
>>> I can reproduce it with the following query on a fresh regression
>>> database:
>>>
>>>      insert into public.brintest_multi (float4col) values (real 'nan');
>>>
>>> The branch was at f6162c020c while testing, backtrace below.
>>
>> I couldn't reproduce this, but it reminds me of this one, which we 
>> also had
>> trouble reproducing.
>>
> 
> I can reproduce that just fine - all I had to do was 'make installcheck' 
> and then connect to the regression db and run the insert.
> 
> It seems to be a simple case of confusion in handling of NaN values. We 
> do sort them correctly (by calling float4_lt), but given two values
> 
>    arg1 = nan (0x400000)
>    arg2 = 0.0909090936
> 
> then a simple comparison does not give the expected result
> 
>    (arg1 < arg2)
>    (arg1 == arg2)
>    (arg1 > arg2)
> 
> all evaluate to false, which is why the assert fails. So I guess the 
> distance function for float4 (and probably float8 too) need a couple 
> more lines checking NaN.
> 

Here's a patch that should fix this. It simply handled NaN and returns 
distance 0.0 for two NaN values and Infinity for NaN vs. something else. 
I did check what happens with Infinity values, but I think those are 
fine - we can actually calculate distance with them, and the assert 
works for them too.

I'll improve the regression tests to include a couple NaN/Infinity cases 
tomorrow, and then I'll get this pushed.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company