From fb234086fe81fb1848934b6e1f6382611fc9ad4f Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Mon, 2 Aug 2021 12:45:01 +0900 Subject: [PATCH] Fix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc. I failed to account for the possibility that when ExecAppendAsyncEventWait() notifies multiple async-capable nodes using postgres_fdw, a preceding node might invoke process_pending_request() to process a pending asynchronous request made by a succeeding node. In that case the succeeding node should produce a tuple to return to the parent Append node from tuples fetched by process_pending_request() when notified. Repair. Per buildfarm via Michael Paquier. Back-patch to v14, like the previous commit. Thanks to Tom Lane for testing. Discussion: https://postgr.es/m/YQP0UPT8KmPiHTMs%40paquier.xyz --- contrib/postgres_fdw/postgres_fdw.c | 22 ++++++++++++++++++---- src/backend/executor/nodeAppend.c | 20 +++++++++++--------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index bd8fda9e91..3bdf7ab182 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -6896,12 +6896,26 @@ postgresForeignAsyncNotify(AsyncRequest *areq) ForeignScanState *node = (ForeignScanState *) areq->requestee; PgFdwScanState *fsstate = (PgFdwScanState *) node->fdw_state; - /* The request should be currently in-process */ - Assert(fsstate->conn_state->pendingAreq == areq); - /* The core code would have initialized the callback_pending flag */ Assert(!areq->callback_pending); + /* + * If process_pending_request() has been invoked on the given request + * before we get here, we might have some tuples already; in which case + * produce the next tuple + */ + if (fsstate->next_tuple < fsstate->num_tuples) + { + produce_tuple_asynchronously(areq, true); + return; + } + + /* We must have run out of tuples */ + Assert(fsstate->next_tuple >= fsstate->num_tuples); + + /* The request should be currently in-process */ + Assert(fsstate->conn_state->pendingAreq == areq); + /* On error, report the original query, not the FETCH. */ if (!PQconsumeInput(fsstate->conn)) pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query); @@ -7027,7 +7041,7 @@ process_pending_request(AsyncRequest *areq) /* * If we didn't get any tuples, must be end of data; complete the request * now. Otherwise, we postpone completing the request until we are called - * from postgresForeignAsyncConfigureWait(). + * from postgresForeignAsyncConfigureWait()/postgresForeignAsyncNotify(). */ if (fsstate->next_tuple >= fsstate->num_tuples) { diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index a4eef19d7f..6a2daa6e76 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -1082,16 +1082,18 @@ ExecAppendAsyncEventWait(AppendState *node) { AsyncRequest *areq = (AsyncRequest *) w->user_data; - /* - * Mark it as no longer needing a callback. We must do this - * before dispatching the callback in case the callback resets the - * flag. - */ - Assert(areq->callback_pending); - areq->callback_pending = false; + if (areq->callback_pending) + { + /* + * Mark it as no longer needing a callback. We must do this + * before dispatching the callback in case the callback resets + * the flag. + */ + areq->callback_pending = false; - /* Do the actual work. */ - ExecAsyncNotify(areq); + /* Do the actual work. */ + ExecAsyncNotify(areq); + } } } }