Re: postgres_fdw: Obsolete comments in GetConnection()

Hi Bharath,

On Wed, Oct 6, 2021 at 6:37 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
> +1 for rewording the comments. Here are my thoughts on the patch:
>
> 1) Just to be consistent(we are using this word in the error message,
> and in other comments around there), how about
> + * Determine whether to try to reestablish the connection.
> instead of
> + * Determine whether to try to remake the connection later.

Actually, we use the word “remake” as well in comments in
connection.c: e.g., “If the connection needs to be *remade* due to
invalidation, disconnect as soon as we're out of all transactions.” in
GetConnection().  But I don’t have a strong opinion about that, so
I’ll change the word as proposed.

> 2) Just to be consistent, how about
> + * cases where we're starting new transaction (not subtransaction),
> if a broken connection is
> instead of
> + * cases where we're out of all transactions, if a broken connection is

Actually, I modified the comment to match existing comments like the
one mentioned above.  I think the patch would actually be more
consistent.

> 3) IMO we don't need the word "later" here because we are immediately
> reestablishing the connection, if it is decided to do so.
> + * Determine whether to try to remake the connection later.

Ok, I’ll drop the word “later”.

Thanks!

Best regards,
Etsuro Fujita