pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
Date: Fri, 23 Jul 2021 16:26:29 +0500
Message-ID: <eb494241-eaa6-cd51-25ea-0b6e3f867d8c@postgrespro.ru> (raw)
In-Reply-To: <CAPmGK170Y-6ysT1Vk+dmz4wXBq6Zq4+Sw2kMywsTdXduA0Svyw@mail.gmail.com>
References: <fe5eaa19-1704-e4a4-76ee-3b9d37ade399@postgrespro.ru>
	<CAPmGK170Y-6ysT1Vk+dmz4wXBq6Zq4+Sw2kMywsTdXduA0Svyw@mail.gmail.com>

On 7/22/21 4:14 PM, Etsuro Fujita wrote:
> On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
> @@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
> 
>      fetch_more_data(node);
> 
> +   /*
> +    * If the request are made by another append we will only prepare connection
> +    * for the next query and don't take a tuple immediately. It is needed to
> +    * prevent possible recursion into a qual subplan.
> +    */
> +   if (!fetch)
> +   {
> +       AppendState *node = (AppendState *) areq->requestor;
> +
> +       ExecAsyncRequestDone(areq, NULL);
> +       node->as_needrequest = bms_add_member(node->as_needrequest,
> +                                             areq->request_index);
> +       return;
> +   }
> 
> I don’t think this is a good idea, because it is pretty inconsistent,
> as doing ExecAsyncRequestDone(areq, NULL) means that there are no more
> tuples while changing as_needrequest like that means that there is at
> least one tuple to return.  This would happen to work, but for
> example, if we add to the core more sanity checks on AsyncRequests,
> this would not work well anymore.  I tried to devise a consistent
> solution for this issue, but I couldn’t.  So I feel inclined to
> disable async execution in cases where async-capable nodes access to
> subplans (or initplans), for now.

That do you think, if (in addition to the patch) we will give 
ExecAsyncAppendResponse a chance to fill the result slot? Code:

inline patch 1: src/backend/executor/nodeAppend.c (+2/-0 lines) download inline-1.patch--- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -1108,6 +1108,9 @@ ExecAsyncAppendResponse(AsyncRequest *areq) return; } + if (areq->result == NULL) + areq->result =
areq->requestee->ExecProcNodeReal(areq->requestee); + /* If the result is NULL or an empty slot, there's nothing more to do. */ if (TupIsNull(slot)) { -- regards, Andrey Lepikhov Postgres Professional

view thread (20+ messages)  latest in thread

Message-ID: <eb494241-eaa6-cd51-25ea-0b6e3f867d8c@postgrespro.ru>
Permalink:  ../eb494241-eaa6-cd51-25ea-0b6e3f867d8c@postgrespro.ru/
Also on:    postgresql.org/message-id/eb494241-eaa6-cd51-25ea-0b6e3f867d8c@postgrespro.ru

 ·  · 

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: pgsql-bugs@postgresql.org
  Cc: a.lepikhov@postgrespro.ru, etsuro.fujita@gmail.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
  In-Reply-To: <eb494241-eaa6-cd51-25ea-0b6e3f867d8c@postgrespro.ru>

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

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