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

I have some review comments on the "Row filter for logical replication" patch:

(1) Suggested update to patch comment:

(There are some missing words and things which could be better expressed)

(2) Some inconsistent error message wording:

Currently:

err = _("cannot use subquery in publication WHERE expression");

Suggest changing it to:

err = _("subqueries are not allowed in publication WHERE expressions");

Other examples from the patch:

err = _("aggregate functions are not allowed in publication WHERE expressions");

err = _("grouping operations are not allowed in publication WHERE expressions");

err = _("window functions are not allowed in publication WHERE expressions");

errmsg("functions are not allowed in publication WHERE expressions"),

err = _("set-returning functions are not allowed in publication WHERE

expressions");

(3) The current code still allows arbitrary code execution, e.g. via a

user-defined operator:

Perhaps add the following after the existing shell error-check in make_op():

/* User-defined operators are not allowed in publication WHERE clauses */

if (pstate->p_expr_kind == EXPR_KIND_PUBLICATION_WHERE && opform->oid

>= FirstNormalObjectId)

    ereport(ERROR,

    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

    errmsg("user-defined operators are not allowed in publication

WHERE expressions"),

    parser_errposition(pstate, location)));

Also, I believe it's also allowing user-defined CASTs (so could add a

similar check to above in transformTypeCast()).

Ideally, it would be preferable to validate/check publication WHERE

expressions in one central place, rather than scattered all over the

place, but that might be easier said than done.

You need to update the patch comment accordingly.

(4) src/backend/replication/pgoutput/pgoutput.c

pgoutput_change()

The 3 added calls to pgoutput_row_filter() are returning from

pgoutput_change(), if false is returned, but instead they should break

from the switch, otherwise cleanup code is missed. This is surely a

bug.