Re: logical replication restrictions

On Wed, Aug 10, 2022, at 9:39 AM, osumi(dot)takamichi(at)fujitsu(dot)com wrote:
> Minor review comments for v6.
Thanks for your review. I'm attaching v7.

> "If the subscriber sets min_apply_delay parameter, ..."
>
> I suggest we use subscription rather than subscriber, because
> this parameter refers to and is used for one subscription.
> My suggestion is
> "If one subscription sets min_apply_delay parameter, ..."
> In case if you agree, there are other places to apply this change.
I changed the terminology to subscription. I also checked other "subscriber"
occurrences but I don't think it should be changed. Some of them are used as
publisher/subscriber pair. If you think there is another sentence to consider,
point it out.

> It might be better to write a note for committer
> like "Bump catalog version" at the bottom of the commit message.
It is a committer task to bump the catalog number. IMO it is easy to notice
(using a git hook?) that it must bump it when we are modifying the catalog.
AFAICS there is no recommendation to add such a warning.

> The former interprets input number as milliseconds in case of no units,
> while the latter takes it as seconds without units.
> I feel it would be better to make them aligned.
In a previous version I decided not to add a code to attach a unit when there
isn't one. Instead, I changed the documentation to reflect what interval_in
uses (seconds as unit). Under reflection, let's use ms as default unit if the
user doesn't specify one.

I fixed all the other suggestions too.

--
Euler Taveira
EDB https://www.enterprisedb.com/