Re: Defer selection of asynchronous subplans until the executor initialization stage

On Sun, Apr 17, 2022 at 1:48 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:

> On Mon, Apr 11, 2022 at 11:44 AM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
> > On Sun, Apr 10, 2022 at 7:41 PM Justin Pryzby <pryzby(at)telsasoft(dot)com>
> wrote:
> >> On Sun, Apr 10, 2022 at 06:46:25AM -0700, Zhihong Yu wrote:
> >> > Looking at the second hunk of the patch:
> >> > FdwRoutine *fdwroutine = path->parent->fdwroutine;
> >> > ...
> >> > + if (IsA(plan, Result))
> >> > + return false;
> >> >
> >> > It seems the check of whether plan is a Result node can be lifted
> ahead of
> >> > the switch statement (i.e. to the beginning of
> mark_async_capable_plan).
> >> >
> >> > This way, we don't have to check for every case in the switch
> statement.
> >>
> >> I think you misread it - the other branch says: if (*not* IsA())
> >>
> > No, I didn't misread:
> >
> > if (!IsA(plan, Result) &&
> > mark_async_capable_plan(plan,
> > ((ProjectionPath *)
> path)->subpath))
> > return true;
> > return false;
> >
> > If the plan is Result node, false would be returned.
> > So the check can be lifted to the beginning of the func.
>
> I think we might support more cases in the switch statement in the
> future. My concern about your proposal is that it might make it hard
> to add new cases to the statement. I agree that what I proposed has a
> bit of redundant code, but writing code inside each case independently
> would make it easy to add them, making code consistent across branches
> and thus making back-patching easy.
>
> Thanks for reviewing!
>
> Best regards,
> Etsuro Fujita
>
Hi,
When a new case arises where the plan is not a Result node, this func can
be rewritten.
If there is only one such new case, the check at the beginning of the func
can be tuned to exclude that case.

I still think the check should be lifted to the beginning of the func
(given the current cases).

Cheers