Hi,
On Wed, Apr 20, 2022 at 4:55 AM David Zhang <david.zhang@highgo.ca> wrote:
> I tried to apply the patch to master and plan to run some tests, but got
> below errors due to other commits.
I rebased the patch against HEAD. Attached is an updated patch.
> + * remote server in parallel at (sub)transaction end.
>
> Here, I think the comment above could potentially apply to multiple
> remote server(s).
I agree on that point, but I think it's correct to say "the remote
server" here, because we determine this for the given remote server.
Maybe I'm missing something, so could you elaborate on it?
> Not sure if there is a way to avoid repeated comments? For example, the
> same comment below appears in two places (line 231 and line 296).
>
> + /*
> + * If requested, consume whatever data is available from the socket.
> + * (Note that if all data is available, this allows
> + * pgfdw_get_cleanup_result to call PQgetResult without forcing the
> + * overhead of WaitLatchOrSocket, which would be large compared to the
> + * overhead of PQconsumeInput.)
> + */
IMO I think it's OK to have this in multiple places, because 1) IMO it
wouldn't be that long, and 2) we already duplicated comments like this
in the same file in v14 and earlier. Here is such an example in
pgfdw_xact_callback() and pgfdw_subxact_callback() in that file in
those versions:
/*
* If a command has been submitted to the remote server by
* using an asynchronous execution function, the command
* might not have yet completed. Check to see if a
* command is still being processed by the remote server,
* and if so, request cancellation of the command.
*/
Thanks for reviewing! Sorry for the delay.
Best regards,
Etsuro Fujita
