RE: Time delayed LR (WAS Re: logical replication restrictions) - Mailing list pgsql-hackers

On Thurs, Feb 2, 2023 16:04 PM Takamichi Osumi (Fujitsu) <osumi.takamichi@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