Some review comments for v50-0004.

======
src/bin/pg_dump/pg_dump.c

dumpSubscription:

1.
+ * We skip the default value ('log') to match the handling of other
+ * default subscription options.
+ */

There is only one "default", so I think this should say "...other
subscription options", not "...other default subscription options".

======
src/include/catalog/pg_subscription.h

2.
+#define CONFLICT_LOG_RELATION_NAME_FMT "pg_conflict_log_%u"
+

2a.
This format string (and first usage of it) really be in one of the
earlier patches.

~

2b.
Add a comment to explain that the %u is the subscription oid.

~

2c.
Is this the correct header file for this? IMO it is CLT specific, so
belongs in conflict.h.

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


Reply via email to