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.von...@2ndquadrant.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...@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)
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
