public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'Euler Taveira' <[email protected]>
To: Melih Mutlu <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Marcos Pegoraro <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Peter Smith <[email protected]>
Subject: RE: logical replication restrictions
Date: Wed, 10 Aug 2022 12:39:29 +0000
Message-ID: <TYWPR01MB83628B9AE512989DCEC67F3FED659@TYWPR01MB8362.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <CAB-JLwYOYwL=XTyAXKiH5CtM_Vm8KjKh7aaitCKvmCh4rzr5pQ@mail.gmail.com>
<CAA4eK1JW+_Pff3TOaBqbOqn-LbMT2kj2bpw-a88ZSTdbTZ-aMg@mail.gmail.com>
<CAB-JLwas_=FGr8bXKD7K6g0=RDUFuYU71TLLGTsQ-DTRNpq1QQ@mail.gmail.com>
<CAA4eK1KSBGrLKR63iS38Bkvjq6fBCWzbuD6R15kEWdmd8aj3mw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAGPVpCQgE1e0JQFHbxSpfj82HQfAqGmhG=yru3mX2tcmbfqHYA@mail.gmail.com>
<[email protected]>
On Tuesday, August 9, 2022 6:47 AM Euler Taveira <[email protected]> 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
view thread (99+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: logical replication restrictions
In-Reply-To: <TYWPR01MB83628B9AE512989DCEC67F3FED659@TYWPR01MB8362.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox