On Tuesday, August 9, 2022 6:47 AM Euler Taveira <eu...@eulerto.com> wrote: > I attached a v6. Hi, thank you for posting the updated patch.
Minor review comments for v6. (1) commit message "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. (2) commit message It might be better to write a note for committer like "Bump catalog version" at the bottom of the commit message. (3) unit alignment between recovery_min_apply_delay and min_apply_delay 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. (4) catalogs.sgml + Delay the application of changes by a specified amount of time. The + unit is in milliseconds. As a column explanation, it'd be better to use a noun in the first sentence to make this description aligned with other places. My suggestion is "Application delay of changes by ....". (5) pg_subscription.c There is one missing blank line before writing if statement. It's written in the AlterSubscription for other cases. @@ -1100,6 +1130,12 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, replaces[Anum_pg_subscription_subdisableonerr - 1] = true; } + if (IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY)) (6) tab-complete.c The order of tab-complete parameters listed in the COMPLETE_WITH should follow alphabetical order. "min_apply_delay" can come before "origin". We can refer to d547f7c commit. (7) 032_apply_delay.pl There are missing whitespaces after comma in the mod functions. UPDATE test_tab SET b = md5(b) WHERE mod(a,2) = 0; DELETE FROM test_tab WHERE mod(a,3) = 0; Best Regards, Takamichi Osumi