From: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Greg Nancarrow <gregn4422(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, Alexey Lesovsky <lesovsky(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | RE: Skipping logical replication transactions on subscriber side |
Date: | 2021-09-02 11:37:04 |
Message-ID: | OS0PR01MB571627030CE342B9E7E25CF094CE9@OS0PR01MB5716.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
From Mon, Aug 30, 2021 3:07 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> I've attached rebased patches. 0004 patch is not the scope of this
> patch. It's borrowed from another thread[1] to fix the assertion
> failure for newly added tests. Please review them.
Hi,
I reviewed the 0002 patch and have a suggestion for it.
+ if (IsSet(opts.specified_opts, SUBOPT_SYNCHRONOUS_COMMIT))
+ {
+ values[Anum_pg_subscription_subsynccommit - 1] =
+ CStringGetTextDatum("off");
+ replaces[Anum_pg_subscription_subsynccommit - 1] = true;
+ }
Currently, the patch set the default value out of parse_subscription_options(),
but I think It might be more standard to set the value in
parse_subscription_options(). Like:
if (!is_reset)
{
...
+ }
+ else
+ opts->synchronous_commit = "off";
And then, we can set the value like:
values[Anum_pg_subscription_subsynccommit - 1] =
CStringGetTextDatum(opts.synchronous_commit);
Besides, instead of adding a switch case like the following:
+ case ALTER_SUBSCRIPTION_RESET_OPTIONS:
+ {
We can add a bool flag(isReset) in AlterSubscriptionStmt and check the flag
when invoking parse_subscription_options(). In this approach, the code can be
shorter.
Attach a diff file based on the v12-0002 which change the code like the above
suggestion.
Best regards,
Hou zj
Attachment | Content-Type | Size |
---|---|---|
0001-diff-for-0002_patch | application/octet-stream | 4.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2021-09-02 11:42:03 | Re: Added schema level support for publication. |
Previous Message | Ajin Cherian | 2021-09-02 11:33:22 | Re: [BUG]Update Toast data failure in logical replication |