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

[wangw.f...@fujitsu.com]

On Thurs, Feb 2, 2023 16:04 PM Takamichi Osumi (Fujitsu) 
<osumi.takami...@fujitsu.com> wrote:
> Attached the updated patch v26 accordingly.

Thanks for your patch.

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)
```

Regards,
Wang Wei