On Mon, Jul 4, 2022 at 1:46 PM shiy.f...@fujitsu.com
<shiy.f...@fujitsu.com> wrote:
>
> On Sun, Jul 3, 2022 11:00 PM vignesh C <vignes...@gmail.com> 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);
Thanks for the comment, I have modified this in the v28 patch attached at [1].
[1] -
https://www.postgresql.org/message-id/CALDaNm3MNK2hMYroTiHGS9HkSxiA-az1QC1mpa0YwDZ8nGmmZg%40mail.gmail.com
Regards,
Vignesh
