On Mon, Dec 28, 2021 9:03 PM houzj.f...@fujitsu.com <houzj.f...@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

Reply via email to