Re: [16+] subscription can end up in inconsistent state - Mailing list pgsql-bugs

Here are some review comments for v4-0001

======
1. GENERAL - terminology

AFAICT from existing documentation, things like 'password' are
referred to as "parameters" of the connection string (i.e. not
"options" of the connection string), so most review comments below are
just repetitions of this point to make all the wording consistent.

======
Commit Message

2.
When the 'password_required' subscription parameter is true, a
non-superuser is only allowed to specify the password via the password
option of connection string, not using a PGPASS file or PGPASSWORD
environment variable.

~

(change the term and add the missing word 'the')

/the password option of connection string/the password parameter of
the connection string/

======
doc/src/sgml/ref/create_subscription.sgml

3.
          <para>
           Specifies whether connections to the publisher made as a result
-          of this subscription must use password authentication. This setting
-          is ignored when the subscription is owned by a superuser.
-          The default is <literal>true</literal>. Only superusers can set
-          this value to <literal>false</literal>.
+          of this subscription must use password authentication. If
+          <literal>true</literal>, a non-superuser is only allowed to specify
+          the password via the password option in connection string. This
+          setting is ignored when the subscription is owned by a superuser. The
+          default is <literal>true</literal>. Only superusers can set this
+          value to <literal>false</literal>.
          </para>

3a.

(same wording as commit message)

/the password option in connection string/the password parameter of
the connection string/

~

3b.
Also, consider using a link, or at least a <literal> for that
"password" parameter.

======
src/backend/replication/libpqwalreceiver/libpqwalreceiver.c

4. libpqrcv_connect

+ /*
+ * A non-superuser is only allowed to specify the password via the
+ * password option in connection string, not using a PGPASS file or
+ * PGPASSWORD environment variable.
+ */

(same wording as commit message)

/the password option in connection string/the password parameter of
the connection string/

======
src/test/subscription/t/027_nosuperuser.pl

5.
+isnt($ret, 0,
+ "non zero exit for subscription whose owner is a non-superuser must
specify password through connection string"
+);

(same wording as commit message)

/password through connection string/password parameter of the connection string/

~~~

6.
+is( $stderr, 'psql:<stdin>:3: ERROR:  password is required
+DETAIL:  Non-superusers must provide a password in the connection string.',
+ 'subscription whose owner is a non-superuser must specify password
through connection string'

(same wording as commit message)

/password through connection string/password parameter of the connection string/

~~~

7.
+# It should be successful after including the password in connection string

/It should be successful/It should succeed/

~

(same wording as commit message)

/password in connection string/password parameter of the connection string/

~~~

8.
+is($ret, 0,
+ "Non-superuser will be able to refresh the publication when the
password is specified in connection string"
+);

(similar wording as commit message)

/when the password is specified in connection string/after specifying
the password parameter of the connection string/


======
Kind Regards,
Peter Smith.
Fujitsu Australia