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