public inbox for [email protected]  

help / color / mirror / Atom feed

From: Etsuro Fujita <[email protected]>
To: Zhihong Yu <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Alexander Pyhalov <[email protected]>
Cc: Andrey V. Lepikhov <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Defer selection of asynchronous subplans until the executor initialization stage
Date: Sun, 17 Apr 2022 17:49:55 +0900
Message-ID: <CAPmGK16tXH6rnQir-bzRb2JTKThURy-HFfbRoGUWv79x5KeQfg@mail.gmail.com> (raw)
In-Reply-To: <CALNJ-vTdGQ-AAM8k7DxAdADYVRFR3FOiyb9ZshkWwyYq2-nQew@mail.gmail.com>
References: <CALNJ-vRF1fw9dh-2sZW9bETqwcttOMw4ptLp6sOrzVUSw=Ycvw@mail.gmail.com>
	<CAPmGK15NkuaVo0Fu_0TfoCpPPJaJi4OMLzEQtkE6Bt6YT52fPQ@mail.gmail.com>
	<[email protected]>
	<CALNJ-vTdGQ-AAM8k7DxAdADYVRFR3FOiyb9ZshkWwyYq2-nQew@mail.gmail.com>

On Mon, Apr 11, 2022 at 11:44 AM Zhihong Yu <[email protected]> wrote:
> On Sun, Apr 10, 2022 at 7:41 PM Justin Pryzby <[email protected]> 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

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Defer selection of asynchronous subplans until the executor initialization stage
  In-Reply-To: <CAPmGK16tXH6rnQir-bzRb2JTKThURy-HFfbRoGUWv79x5KeQfg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox