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
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?
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."