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

Zhihong Yu <zyu@yugabyte.com>

From: Zhihong Yu <zyu@yugabyte.com>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Alexander Pyhalov <a.pyhalov@postgrespro.ru>, "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2022-04-17T10:34:55Z
Lists: pgsql-hackers
On Sun, Apr 17, 2022 at 1:48 AM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

> On Mon, Apr 11, 2022 at 11:44 AM Zhihong Yu <zyu@yugabyte.com> wrote:
> > On Sun, Apr 10, 2022 at 7:41 PM Justin Pryzby <pryzby@telsasoft.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

Commits

  1. Improve comments for trivial_subqueryscan().

  2. Disable asynchronous execution if using gating Result nodes.

  3. Allow asynchronous execution in more cases.

  4. Doc: Further update documentation for asynchronous execution.

  5. Fix rescanning of async-aware Append nodes.

  6. Doc: Update documentation for asynchronous execution.

  7. Fix EXPLAIN ANALYZE for async-capable nodes.

  8. Minor code cleanup in asynchronous execution support.

  9. Adjust input value to WaitEventSetWait() in ExecAppendAsyncEventWait().

  10. Add support for asynchronous execution.