RE: Handle infinite recursion in logical replication setup

On Wed, May 4, 2022 2:47 PM vignesh C <vignesh21@gmail.com> wrote:
> 
> Thanks for the comments, the attached v13 patch has the changes for the same.
>

Here are some comments on v13-0002 patch.

1)
+         * Throw an error so that the user can take care of the initial data
+         * copying and then create subscription with copy_data off.

Should "with copy_data off" be changed to "with copy_data off or force"?

2)
        case ALTER_SUBSCRIPTION_ADD_PUBLICATION:
        case ALTER_SUBSCRIPTION_DROP_PUBLICATION:
...
                    /*
                     * See ALTER_SUBSCRIPTION_REFRESH for details why this is
                     * not allowed.
                     */
                    if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && opts.copy_data)

I think we need some changes here, too. Should it be modified to:
if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && IS_COPY_DATA_ON_OR_FORCE(opts.copy_data))

Regards,
Shi yu