On Thu, Jun 16, 2022 at 3:48 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, Jun 15, 2022 at 12:09 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> Thanks for the comments, the attached v21 patch has the changes for the same.
I have done some basic review of v21 and I have a few comments,
1.
+/*
+ * The subscription will request the publisher to only send changes that
+ * originated locally.
+ */
+#define LOGICALREP_ORIGIN_LOCAL "local"
+
+/*
+ * The subscription will request the publisher to send any changes regardless
+ * of their origin
+ */
+#define LOGICALREP_ORIGIN_ANY "any"
Are we planning to extend this to more options or are we planning to
support the actual origin name here? If not then why isn't it just
bool? I think the comments and the patch commit message should
explain the details behind it if it has been already discussed and
concluded.
2.
+/*
+ * Check and throw an error if the publisher has subscribed to the same table
+ * from some other publisher. This check is required only if copydata is ON and
+ * the origin is local.
+ */
I think it should also explain why this combination is not allowed and
if it is already explained in code
then this code can add comments to refer to that part of the code.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com