On 2024-07-08 15:28, Fujii Masao wrote:
On 2024/07/01 18:15, Jelte Fennema-Nio wrote:
On Thu, 27 Jun 2024 at 12:27, ikedarintarof
<ikedarinta...@oss.nttdata.com> wrote:
Thanks for your suggestion. I used ChatGPT to choose the wording, but
it's still difficult for me.
Looks good to me now (but obviously biased since you took my wording).
LGTM, too.
* Validate connection info string, and determine whether it might
cause
* local filesystem access to be attempted.
The later part of the above comment for libpqrcv_check_conninfo()
seems unclear. My understanding is that if must_use_password is true
and this function completes without error, the password must be
in the connection string, so there's no need to read the .pgpass file
(i.e., no local filesystem access). That part seems to be trying to
explaing something like that. Is this correct?
Anyway, wouldn't it be better to either remove this unclear part or
rephrase it for clarity?
Regards,
Thank you for your comment.
I agree "local filesystem access" is vague. I think the reference to
.pgpass
(local file system) is not necessary in the comment for
libpqrcv_check_conninfo()
because it is explained in libpqrcv_connect() as shown below.
/*
* Re-validate connection string. The validation already happened at DDL
* time, but the subscription owner may have changed. If we don't
recheck
* with the correct must_use_password, it's possible that the connection
* will obtain the password from a different source, such as PGPASSFILE
or
* PGPASSWORD.
*/
libpqrcv_check_conninfo(conninfo, must_use_password);
I remove the unclear part from the previous patch and add some
explanation for
later part of the function.
Or, I think it is also good to make the comment more simple:
* The function checks that
* 1. connection info string is properly parsed and
* 2. a password is provided if must_use_password is true.
* If the check is failed, the it will raise an error.
*/
static void
libpqrcv_check_conninfo(const char *conninfo, bool must_use_password)
Regards,
Rintaro Ikeda
From 514bd81f7d1b7104e6d2d92cb072a84096cd3885 Mon Sep 17 00:00:00 2001
From: Rintaro Ikeda <51394766+rinik...@users.noreply.github.com>
Date: Thu, 27 Jun 2024 16:04:08 +0900
Subject: [PATCH v2] modify the commnet in libpqrcv_check_conninfo()
---
.../replication/libpqwalreceiver/libpqwalreceiver.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 02f12f2921..e6a4e4cc1c 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -305,12 +305,14 @@ bad_connection:
}
/*
- * Validate connection info string, and determine whether it might cause
- * local filesystem access to be attempted.
+ * The function
+ * 1. validates connection info string and
+ * 2. checks a password is provided if must_use_password is true.
*
* If the connection string can't be parsed, this function will raise
- * an error and will not return. If it can, it will return true if this
- * connection string specifies a password and false otherwise.
+ * an error. If must_use_password is true, the function raises an error
+ * if no password is provided in the connection string. In any other case
+ * it successfully completes.
*/
static void
libpqrcv_check_conninfo(const char *conninfo, bool must_use_password)
--
2.39.3 (Apple Git-146)
