Hi,
On Wed, Feb 28, 2024 at 02:23:27AM +0000, Zhijie Hou (Fujitsu) wrote:
> Attach the V100 patch set which addressed above comments.
Thanks!
A few random comments:
1 ===
+ if (!ok)
+ {
+ GUC_check_errdetail("List syntax is invalid.");
+ }
What about to get rid of the brackets here?
2 ===
+
+ /*
+ * If the replication slots' data have been initialized, verify if the
+ * specified slots exist and are logical slots.
+ */
remove the empty line above the comment?
3 ===
+check_standby_slot_names(char **newval, void **extra, GucSource source)
+{
+ if ((*newval)[0] == '\0')
+ return true;
I think "**newval == '\0'" is easier to read but that's a matter of taste and
check_synchronous_standby_names() is already using the same so it's a nit.
4 ===
Regarding the test, what about adding one to test the "new" behavior discussed
up-thread? (logical replication will wait if slot mentioned in
standby_slot_names
is dropped and/or does not exist when the engine starts?)
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
