Re: row filtering for logical replication

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