On Thu, Dec 10, 2020 at 5:00 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Thu, Dec 10, 2020 at 4:49 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
> > > + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
> > > + plannedstmt->parallelModeNeeded &&
> > > + plannedstmt->planTree &&
> > > + IsA(plannedstmt->planTree, Gather) &&
> > > + plannedstmt->planTree->lefttree &&
> > > +
> > > plannedstmt->planTree->lefttree->parallel_aware &&
> > > +
> > > plannedstmt->planTree->lefttree->parallel_safe;
> > >
> > > I noticed it check both IsA(ps, GatherState) and
> > > IsA(plannedstmt->planTree, Gather).
> > > Does it mean it is possible that IsA(ps, GatherState) is true but
> > > IsA(plannedstmt->planTree, Gather) is false ?
> > >
> > > I did some test but did not find a case like that.
> > >
> >
> > This seems like an extra check. Apart from that if we combine 0001
> > and 0002 there should be an additional protection so that it should
> > not happen that in cost_gather we have ignored the parallel tuple cost
> > and now we are rejecting the parallel insert. Probably we should add
> > an assert.
>
> Yeah it's an extra check. I don't think we need that extra check
> IsA(plannedstmt->planTree, Gather). GatherState check is enough. I verified
> it as follows: the gatherstate will be allocated and initialized with the
> plan tree in ExecInitGather which are the ones we are checking here. So,
> there is no chance that the plan state is GatherState and the plan tree will
> not be Gather. I will remove IsA(plannedstmt->planTree, Gather) check in the
> next version of the patch set.
>
> Breakpoint 4, ExecInitGather (node=0x5647f98ae994 <ExecCheckRTEPerms+131>,
> estate=0x1ca8, eflags=730035099) at nodeGather.c:61
> (gdb) p gatherstate
> $10 = (GatherState *) 0x5647fac83850
> (gdb) p gatherstate->ps.plan
> $11 = (Plan *) 0x5647fac918a0
>
> Breakpoint 1, IsParallelInsertInCTASAllowed (into=0x5647fac97580,
> queryDesc=0x5647fac835e0) at createas.c:663
> 663 {
> (gdb) p ps
> $13 = (PlanState *) 0x5647fac83850
> (gdb) p ps->plan
> $14 = (Plan *) 0x5647fac918a0
>
Hope you did not miss the second part of my comment
"
> Apart from that if we combine 0001
> and 0002 there should be additional protection so that it should
> not happen that in cost_gather we have ignored the parallel tuple cost
> and now we are rejecting the parallel insert. Probably we should add
> an assert.
"
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
