Greg:
bq. we should just return parsetree->hasModifyingCTE at this point,

Maybe you can clarify a bit.
The if (parsetree->hasModifyingCTE) check is followed by if
(!hasModifyingCTE).
When parsetree->hasModifyingCTE is false, !hasModifyingCTE would be true,
resulting in the execution of the if (!hasModifyingCTE) block.

In your reply, did you mean that the if (!hasModifyingCTE) block is no
longer needed ? (I guess not)

Cheers

On Thu, Feb 11, 2021 at 8:14 PM Greg Nancarrow <gregn4...@gmail.com> wrote:

> On Fri, Feb 12, 2021 at 2:33 PM Zhihong Yu <z...@yugabyte.com> wrote:
> >
> > For v17-0001-Enable-parallel-SELECT-for-INSERT-INTO-.-SELECT.patch :
> >
> > +       /* Assume original queries have hasModifyingCTE set correctly */
> > +       if (parsetree->hasModifyingCTE)
> > +           hasModifyingCTE = true;
> >
> > Since hasModifyingCTE is false by the time the above is run, it can be
> simplified as:
> >     hasModifyingCTE = parsetree->hasModifyingCTE
> >
>
> Actually, we should just return parsetree->hasModifyingCTE at this
> point, because if it's false, we shouldn't need to continue the search
> (as we're assuming it has been set correctly for QSRC_ORIGINAL case).
>
> > +   if (!hasSubQuery)
> > +       return false;
> > +
> > +   return true;
> >
> > The above can be simplified as:
> >     return hasSubQuery;
> >
>
> Yes, absolutely right, silly miss on that one!
> Thanks.
>
> This was only ever meant to be a temporary fix for this bug that
> affects this patch.
>
> Regards,
> Greg Nancarrow
> Fujitsu Australia
>

Reply via email to