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