Re: Asynchronous Append on postgres_fdw nodes.

[Etsuro Fujita]

On Fri, May 7, 2021 at 7:32 PM Andrey Lepikhov
<a.lepik...@postgrespro.ru> wrote:
> Ok, I agree with the approach, but the next test case failed:
>
> EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
> SELECT * FROM (
>         (SELECT * FROM f1) UNION ALL (SELECT * FROM f2)
> ) q1 LIMIT 100;
> ERROR:  InstrUpdateTupleCount called on node not yet executed
>
> Initialization script see in attachment.

Reproduced.  Here is the EXPLAIN output for the query:

explain verbose select * from ((select * from f1) union all (select *
from f2)) q1 limit 100;
                                      QUERY PLAN
--------------------------------------------------------------------------------------
 Limit  (cost=100.00..104.70 rows=100 width=4)
   Output: f1.a
   ->  Append  (cost=100.00..724.22 rows=13292 width=4)
         ->  Async Foreign Scan on public.f1  (cost=100.00..325.62
rows=6554 width=4)
               Output: f1.a
               Remote SQL: SELECT a FROM public.l1
         ->  Async Foreign Scan on public.f2  (cost=100.00..332.14
rows=6738 width=4)
               Output: f2.a
               Remote SQL: SELECT a FROM public.l2
(9 rows)

When executing the query “select * from ((select * from f1) union all
(select * from f2)) q1 limit 100” in async mode, the remote queries
for f1 and f2 would be sent to the remote at the same time in the
first ExecAppend().  If the result for the remote query for f1 is
returned first, the local query would be processed using the result,
and the remote query for f2 in progress would be processed during
ExecutorEnd() using process_pending_request() (and vice versa).  But
in the EXPLAIN ANALYZE case, InstrEndLoop() is called *before*
ExecutorEnd(), and it initializes the instr->running flag, so in that
case, when processing the in-progress remote query in
process_pending_request(), we would call InstrUpdateTupleCount() with
the flag unset, causing this error.

I think a simple fix for this would be just remove the check whether
the instr->running flag is set or not in InstrUpdateTupleCount().
Attached is an updated patch, in which I also updated a comment in
execnodes.h and docs in fdwhandler.sgml to match the code in
nodeAppend.c, and fixed typos in comments in nodeAppend.c.

Thanks for the review and script!

Best regards,
Etsuro Fujita

fix-EXPLAIN-ANALYZE-for-async-capable-nodes-v2.patch
Description: Binary data