Re: Parallel INSERT (INTO ... SELECT ...) - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: Parallel INSERT (INTO ... SELECT ...)
Date
Msg-id CALNJ-vQ7LjYJaRjqC-V3YvHNkgVB=z=pOWHYdisfZ+38yVM08g@mail.gmail.com
Whole thread Raw
In response to Re: Parallel INSERT (INTO ... SELECT ...)  (Greg Nancarrow <gregn4422@gmail.com>)
Responses Re: Parallel INSERT (INTO ... SELECT ...)
List pgsql-hackers
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 <gregn4422@gmail.com> wrote:
On Fri, Feb 12, 2021 at 2:33 PM Zhihong Yu <zyu@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

pgsql-hackers by date:

Previous
From: Greg Nancarrow
Date:
Subject: Re: Parallel INSERT (INTO ... SELECT ...)
Next
From: Yugo NAGATA
Date:
Subject: Re: Is Recovery actually paused?