RE: row filtering for logical replication

On Mon, Dec 28, 2021 9:03 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> Attach a top up patch 0004 which did the above changes.

A few comments about v55-0001 and v55-0002.
v55-0001
1.
There is a typo at the last sentence of function(rowfilter_walker)'s comment.
* (b) a user-defined function can be used to access tables which could have
* unpleasant results because a historic snapshot is used. That's why only
- * non-immutable built-in functions are allowed in row filter expressions.
+ * immutable built-in functions are allowed in row filter expressions.

2.
There are two if statements at the end of fetch_remote_table_info.
+ if (!isnull)
+ *qual = lappend(*qual, makeString(TextDatumGetCString(rf)));
+
+ ExecClearTuple(slot);
+
+ /* Ignore filters and cleanup as necessary. */
+ if (isnull)
+ {
+ if (*qual)
+ {
+ list_free_deep(*qual);
+ *qual = NIL;
+ }
+ break;
+ }
What about using the format like following:
if (!isnull)
...
else
...

v55-0002
In function pgoutput_row_filter_init, I found almost whole function is in the if
statement written like this:
static void
pgoutput_row_filter_init()
{
Variable declaration and initialization;
if (!entry->exprstate_valid)
{
......
}
}
What about changing this if statement like following:
if (entry->exprstate_valid)
return;

Regards,
Wang wei