On Sun, Jul 3, 2022 11:00 PM vignesh C <[email protected]> wrote:
>
> Thanks for the comments, the attached v27 patch has the changes for the
> same.
>
Thanks for updating the patch.
A comment on 0003 patch:
+ /*
+ * No need to throw an error for the tables that are in ready
state,
+ * as the walsender will send the changes from WAL in case of
tables
+ * in ready state.
+ */
+ if (isreadytable)
+ continue;
+
...
+ ereport(ERROR,
+
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("table: \"%s.%s\" might have replicated
data in the publisher",
+ nspname, relname),
+ errdetail("CREATE/ALTER SUBSCRIPTION with
origin = local and copy_data = on is not allowed when the publisher might have
replicated data."),
+ errhint("Use CREATE/ALTER SUBSCRIPTION with
copy_data = off/force."));
+
+ ExecClearTuple(slot);
I think we should call ExecClearTuple() before getting next tuple, so it should
be called if the table is in ready state. How about modifying it to:
if (!isreadytable)
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("table: \"%s.%s\" might have
replicated data in the publisher",
nspname, relname),
errdetail("CREATE/ALTER SUBSCRIPTION
with origin = local and copy_data = on is not allowed when the publisher might
have replicated data."),
errhint("Use CREATE/ALTER SUBSCRIPTION
with copy_data = off/force."));
ExecClearTuple(slot);
Regards,
Shi yu
