RE: row filtering for logical replication - Mailing list pgsql-hackers

From wangw.fnst@fujitsu.com
Subject RE: row filtering for logical replication
Date
Msg-id OS3PR01MB62756D18BA0FA969D5255E369E459@OS3PR01MB6275.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: row filtering for logical replication  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
Responses Re: row filtering for logical replication  (Peter Smith <smithpb2250@gmail.com>)
List pgsql-hackers
On Mon, Dec 28, 2021 9:03 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.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

pgsql-hackers by date:

Previous
From: SATYANARAYANA NARLAPURAM
Date:
Subject: Re: Throttling WAL inserts when the standby falls behind more than the configured replica_lag_in_bytes
Next
From: Peter Eisentraut
Date:
Subject: Re: Add Boolean node