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

Hi,

I am interested in this feature and took a quick a look at the patch.
Here are a few comments.

(1)
+                appendStringInfo(&cmd, "%s", q);

We'd better use appendStringInfoString(&cmd, q);


(2)
+    whereclause = transformWhereClause(pstate,
+                                       copyObject(pri->whereClause),
+                                       EXPR_KIND_PUBLICATION_WHERE,
+                                       "PUBLICATION");
+
+    /* Fix up collation information */
+    assign_expr_collations(pstate, whereclause);

Is it better to invoke eval_const_expressions or canonicalize_qual here to
simplify the expression ?


(3)
+                appendPQExpBuffer(&buf,
+                                  ", pg_get_expr(pr.prqual, c.oid)");
+            else
+                appendPQExpBuffer(&buf,
+                                  ", NULL");

we'd better use appendPQExpBufferStr instead of appendPQExpBuffer here.

(4)
nodeTag(expr) == T_FuncCall)

It might looks clearer to use IsA(expr, FuncCall) here.

Best regards,
Houzj