Re: Time delayed LR (WAS Re: logical replication restrictions)

On Fri, Feb 3, 2023 at 3:12 PM wangw.fnst@fujitsu.com
<wangw.fnst@fujitsu.com> wrote:
>
> Here is a comment:
>
> 1. The checks in function AlterSubscription
> +                                       /*
> +                                        * The combination of parallel streaming mode and
> +                                        * min_apply_delay is not allowed. See
> +                                        * parse_subscription_options for details of the reason.
> +                                        */
> +                                       if (opts.streaming == LOGICALREP_STREAM_PARALLEL)
> +                                               if ((IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) &&
opts.min_apply_delay> 0) ||
 
> +                                                       (!IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) &&
sub->minapplydelay> 0))
 
> and
> +                                       /*
> +                                        * The combination of parallel streaming mode and
> +                                        * min_apply_delay is not allowed.
> +                                        */
> +                                       if (opts.min_apply_delay > 0)
> +                                               if ((IsSet(opts.specified_opts, SUBOPT_STREAMING) && opts.streaming
==LOGICALREP_STREAM_PARALLEL) ||
 
> +                                                       (!IsSet(opts.specified_opts, SUBOPT_STREAMING) && sub->stream
==LOGICALREP_STREAM_PARALLEL))
 
>
> I think the case where the options "min_apply_delay>0" and "streaming=parallel"
> are set at the same time seems to have been checked in the function
> parse_subscription_options, how about simplifying these two if-statements here
> to the following:
> ```
> if (opts.streaming == LOGICALREP_STREAM_PARALLEL &&
>         !IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) &&
>         sub->minapplydelay > 0)
>
> and
>
> if (opts.min_apply_delay > 0 &&
>         !IsSet(opts.specified_opts, SUBOPT_STREAMING) &&
>         sub->stream == LOGICALREP_STREAM_PARALLEL)
> ```
>

Won't just checking if ((opts.streaming == LOGICALREP_STREAM_PARALLEL
&& sub->minapplydelay > 0) || (opts.min_apply_delay > 0 && sub->stream
== LOGICALREP_STREAM_PARALLEL)) be sufficient in that case?

-- 
With Regards,
Amit Kapila.