From: | Greg Nancarrow <gregn4422(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Smith <smithpb2250(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Önder Kalacı <onderkalaci(at)gmail(dot)com>, japin <japinli(at)hotmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, David Steele <david(at)pgmasters(dot)net>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: row filtering for logical replication |
Date: | 2022-01-21 09:25:52 |
Message-ID: | CAJcOf-eUnXPSDR1smg9VFktr6OY5=8zAsCX-rqctBdfgoEavDA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jan 20, 2022 at 12:12 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Attach the V68 patch set which addressed the above comments and changes.
> The version patch also fix the error message mentioned by Greg[1]
>
Some review comments for the v68 patch, mostly nitpicking:
(1) Commit message
Minor suggested updates:
BEFORE:
Allow specifying row filter for logical replication of tables.
AFTER:
Allow specifying row filters for logical replication of tables.
BEFORE:
If you choose to do the initial table synchronization, only data that
satisfies the row filters is pulled by the subscriber.
AFTER:
If you choose to do the initial table synchronization, only data that
satisfies the row filters is copied to the subscriber.
src/backend/executor/execReplication.c
(2)
BEFORE:
+ * table does not publish UPDATES or DELETES.
AFTER:
+ * table does not publish UPDATEs or DELETEs.
src/backend/replication/pgoutput/pgoutput.c
(3) pgoutput_row_filter_exec_expr
pgoutput_row_filter_exec_expr() returns false if "isnull" is true,
otherwise (if "isnull" is false) returns the value of "ret"
(true/false).
So the following elog needs to be changed (Peter Smith previously
pointed this out, but it didn't get completely changed):
BEFORE:
+ elog(DEBUG3, "row filter evaluates to %s (isnull: %s)",
+ DatumGetBool(ret) ? "true" : "false",
+ isnull ? "true" : "false");
AFTER:
+ elog(DEBUG3, "row filter evaluates to %s (isnull: %s)",
+ isnull ? "false" : DatumGetBool(ret) ? "true" : "false",
+ isnull ? "true" : "false");
(4) pgoutput_row_filter_init
BEFORE:
+ * we don't know yet if there is/isn't any row filters for this relation.
AFTER:
+ * we don't know yet if there are/aren't any row filters for this relation.
BEFORE:
+ * necessary at all. This avoids us to consume memory and spend CPU cycles
+ * when we don't need to.
AFTER:
+ * necessary at all. So this allows us to avoid unnecessary memory
+ * consumption and CPU cycles.
(5) pgoutput_row_filter
BEFORE:
+ * evaluates the row filter for that tuple and return.
AFTER:
+ * evaluate the row filter for that tuple and return.
Regards,
Greg Nancarrow
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | houzj.fnst@fujitsu.com | 2022-01-21 09:39:33 | RE: row filtering for logical replication |
Previous Message | wenjing zeng | 2022-01-21 09:24:51 | Re: [PATCH] Implement INSERT SET syntax |