From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
Cc: | James Coleman <jtc331(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Fix division by zero (explain.c) |
Date: | 2020-05-08 23:20:08 |
Message-ID: | 20200508232008.6n56ni43wyjaitou@development |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, May 08, 2020 at 07:25:36PM -0300, Ranier Vilela wrote:
>Em sex., 8 de mai. de 2020 às 19:02, Tomas Vondra <
>tomas(dot)vondra(at)2ndquadrant(dot)com> escreveu:
>
>> 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(dot)vf(at)gmail(dot)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)
>>
>
>> First this line, contradicts the comments. According to the comments,
>if ( fullsortGroupInfo->groupCount == 0) is true, there is no need to do
>anything else, next.
>So anyway, we don't need to test anything anymore.
>
>Now, to happen the division by zero, (prefixsortGroupInfo->groupCount == 0,
>needs to be true too,
>Maybe this is not happening, but if it happens, it divides by zero, just
>below, so if an unnecessary test and adds a risk, why not, remove it?
>
Well, I'd like to understand what the bug is. If possible, I'd like to
add a test case, for example.
>
>> 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.
>>
>Comments says:
>"we don't need to do anything if there were 0 full groups."
>
True. But it also implies that in order to have prefix groups we need to
have a full group first. Which implies that
(#full == 0) && (#prefix != 0)
is not really possible.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Gurjeet Singh | 2020-05-08 23:31:48 | Re: JSON output from psql |
Previous Message | Tomas Vondra | 2020-05-08 23:14:17 | Re: Incremental sorts and EXEC_FLAG_REWIND |