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
-
Improve comments for trivial_subqueryscan().
- 4a8a5dd7f59c 15.0 landed
-
Disable asynchronous execution if using gating Result nodes.
- ebb790241523 14.3 landed
- 5c854e7a2c8a 15.0 landed
-
Allow asynchronous execution in more cases.
- c2bb02bc2e85 15.0 landed
-
Doc: Further update documentation for asynchronous execution.
- eab81953682d 14.0 landed
-
Fix rescanning of async-aware Append nodes.
- f3baaf28a6da 14.0 landed
-
Doc: Update documentation for asynchronous execution.
- 15fcd33e0694 14.0 landed
-
Fix EXPLAIN ANALYZE for async-capable nodes.
- a363bc6da96b 14.0 landed
-
Minor code cleanup in asynchronous execution support.
- bb684c82f733 14.0 landed
-
Adjust input value to WaitEventSetWait() in ExecAppendAsyncEventWait().
- a8af856d3257 14.0 landed
-
Add support for asynchronous execution.
- 27e1f14563cf 14.0 landed