RE: Handle infinite recursion in logical replication setup

On Wed, May 4, 2022 2:47 PM vignesh C  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