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

On Fri, Jan 22, 2021 at 8:29 AM Greg Nancarrow <gregn4422@gmail.com> wrote:
>
> On Thu, Jan 21, 2021 at 7:30 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > > i.e. code-wise:
> > >
> > >         /*
> > > -        * We can't support table modification in parallel-mode if
> > > it's a foreign
> > > -        * table/partition (no FDW API for supporting parallel access) or a
> > > +        * We can't support table modification in a parallel worker if it's a
> > > +        * foreign table/partition (no FDW API for supporting parallel
> > > access) or a
> > >          * temporary table.
> > >          */
> > >         if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE ||
> > >                 RelationUsesLocalBuffers(rel))
> > >         {
> > > -               table_close(rel, lockmode);
> > > -               context->max_hazard = PROPARALLEL_UNSAFE;
> > > -               return true;
> > > +               if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
> > > +               {
> > > +                       table_close(rel, lockmode);
> > > +                       return true;
> > > +               }
> > >         }
> > >
> >
> > Yeah, these changes look correct to me.
> >
>
> Unfortunately, this change results in a single test failure in the
> "with" tests when "force_parallel_mode=regress" is in effect.
>
> I have reproduced the problem, by extracting relevant SQL from those
> tests, as follows:
>
> CREATE TEMP TABLE bug6051 AS
>   select i from generate_series(1,3) as i;
> SELECT * FROM bug6051;
> CREATE TEMP TABLE bug6051_2 (i int);
> CREATE RULE bug6051_ins AS ON INSERT TO bug6051 DO INSTEAD
>  INSERT INTO bug6051_2
>  SELECT NEW.i;
> WITH t1 AS ( DELETE FROM bug6051 RETURNING * )
> INSERT INTO bug6051 SELECT * FROM t1;
> ERROR:  cannot delete tuples during a parallel operation
>
> Note that prior to the patch, all INSERTs were regarded as
> PARALLEL_UNSAFE, so this problem obviously didn't occur.
> I believe this INSERT should be regarded as PARALLEL_UNSAFE, because
> it contains a modifying CTE.
> However, for some reason, the INSERT is not regarded as having a
> modifying CTE, so instead of finding it PARALLEL_UNSAFE, it falls into
> the parallel-safety-checks and is found to be PARALLEL_RESTRICTED:
>
> The relevant code in standard_planner() is:
>
>     if ((cursorOptions & CURSOR_OPT_PARALLEL_OK) != 0 &&
>         IsUnderPostmaster &&
>         (parse->commandType == CMD_SELECT ||
>          IsModifySupportedInParallelMode(parse->commandType)) &&
>         !parse->hasModifyingCTE &&
>         max_parallel_workers_per_gather > 0 &&
>         !IsParallelWorker())
>     {
>         /* all the cheap tests pass, so scan the query tree */
>         glob->maxParallelHazard = max_parallel_hazard(parse);
>         glob->parallelModeOK = (glob->maxParallelHazard != PROPARALLEL_UNSAFE);
>     }
>     else
>     {
>         /* skip the query tree scan, just assume it's unsafe */
>         glob->maxParallelHazard = PROPARALLEL_UNSAFE;
>         glob->parallelModeOK = false;
>     }
>
> When I debugged this (transformWithClause()), the WITH clause was
> found to contain a modifying CTE and for the INSERT
> query->hasModifyingCTE was set true.
> But somehow in the re-writer code, this got lost.
> Bug?
> Ideas?
>

How it behaves when the table in the above test is a non-temp table
with your patch? If it leads to the same error then we can at least
conclude that this is a generic problem and nothing specific to temp
tables.

-- 
With Regards,
Amit Kapila.