On Thu, Dec 10, 2020 at 3:38 PM Andrey V. Lepikhov
<a.lepik...@postgrespro.ru> wrote:
> On 11/17/20 2:56 PM, Etsuro Fujita wrote:
> > On Mon, Oct 5, 2020 at 3:35 PM Etsuro Fujita <etsuro.fuj...@gmail.com> 
> > wrote:
> > Comments welcome!  The attached is still WIP and maybe I'm missing
> > something, though.
> I reviewed your patch and used it in my TPC-H benchmarks. It is still
> WIP. Will you improve this patch?

Yeah, will do.

> I also want to say that, in my opinion, Horiguchi-san's version seems
> preferable: it is more structured, simple to understand, executor-native
> and allows to reduce FDW interface changes.

I’m not sure what you mean by “executor-native”, but I partly agree
that Horiguchi-san’s version would be easier to understand, because
his version was made so that a tuple is requested from an async
subplan using our Volcano Iterator model almost as-is.  But my
concerns about his version would be: 1) it’s actually pretty invasive,
because it changes the contract of the ExecProcNode() API [1], and 2)
IIUC it wouldn’t allow us to find ready subplans from occurred wait
events when we extend to the case where subplans are joins or
aggregates over ForeignScans [2].

> This code really only needs
> one procedure - IsForeignPathAsyncCapable.

This isn’t correct: his version uses ForeignAsyncConfigureWait() as well.

Thank you for reviewing!  Sorry for the delay.

Best regards,
Etsuro Fujita

[1] 
https://www.postgresql.org/message-id/CAPmGK16YXCADSwsFLSxqTBBLbt3E_%3DiigKTtjS%3Ddqu%2B8K8DWCw%40mail.gmail.com
[2] 
https://www.postgresql.org/message-id/CAPmGK16rA5ODyRrVK9iPsyW-td2RcRZXsdWoVhMmLLmUhprsTg%40mail.gmail.com


Reply via email to