Re: [PATCH] Fix division by zero (explain.c)

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?

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

regards,
Ranier Vilela