On Thu, Jan 20, 2022 at 12:12 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.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
