Re: [PATCH] Fix division by zero (explain.c) - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [PATCH] Fix division by zero (explain.c)
Date
Msg-id 20200508220229.bwoknbfx2hkepzt6@development
Whole thread Raw
In response to Re: [PATCH] Fix division by zero (explain.c)  (James Coleman <jtc331@gmail.com>)
Responses Re: [PATCH] Fix division by zero (explain.c)
List pgsql-hackers
On Thu, Apr 23, 2020 at 04:12:34PM -0400, James Coleman wrote:
>On Thu, Apr 23, 2020 at 8:38 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
>>
>> Hi,
>>
>> Per Coverity.
>>
>> If has 0 full groups, "we don't need to do anything" and need goes to next.
>> Otherwise a integer division by zero, can raise.
>>
>> comments extracted trom explain.c:
>>  /*
>> * Since we never have any prefix groups unless we've first sorted
>> * a full groups and transitioned modes (copying the tuples into a
>> * prefix group), we don't need to do anything if there were 0 full
>> * groups.
>> */
>
>This does look like a fairly obvious thinko on my part, and the patch
>looks correct to me.
>
>Tomas: agreed?
>

So how do we actually get the division by zero? It seems to me the fix
prevents  a division by zero with 0 full groups and >0 prefix groups,
but can that actually happen?

But can that actually happen? Doesn't the comment quoted in the report
actually suggest otherwise? If this

   (fullsortGroupInfo->groupCount == 0 &&
    prefixsortGroupInfo->groupCount == 0)

evaluates to false, and

   (fullsortGroupInfo->groupCount == 0)

this evaluates to true, then clearly there would have to be 0 full
groups and >0 prefix groups. But the comment says that can't happen,
unless I misunderstand what it's saying.

I've tried to trigger the issue, but without success ...


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: refactoring basebackup.c
Next
From: Ranier Vilela
Date:
Subject: Re: [PATCH] Fix division by zero (explain.c)