Re: row filtering for logical replication
От | Euler Taveira |
---|---|
Тема | Re: row filtering for logical replication |
Дата | |
Msg-id | 34b61db2-3e26-42ed-bb84-b14a9fe0a60d@www.fastmail.com обсуждение исходный текст |
Ответ на | Re: row filtering for logical replication (Greg Nancarrow <gregn4422@gmail.com>) |
Ответы |
Re: row filtering for logical replication
(Tomas Vondra <tomas.vondra@enterprisedb.com>)
|
Список | pgsql-hackers |
On Mon, Jul 5, 2021, at 12:14 AM, Greg Nancarrow wrote:
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)
I incorporated all your wording suggestions.
(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");
The same expression "cannot use subquery in ..." is used in the other switch
cases. If you think this message can be improved, I suggest that you submit a
separate patch to change all sentences.
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 WHEREexpressions");
This is a different function. I just followed the same wording from similar
sentences around it.
(3) The current code still allows arbitrary code execution, e.g. via auser-defined operator:
I fixed it in v18.
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 publicationWHERE expressions"),parser_errposition(pstate, location)));
I'm still working on a way to accept built-in functions but while we don't have
it, let's forbid custom operators too.
Also, I believe it's also allowing user-defined CASTs (so could add asimilar check to above in transformTypeCast()).Ideally, it would be preferable to validate/check publication WHEREexpressions in one central place, rather than scattered all over theplace, but that might be easier said than done.You need to update the patch comment accordingly.
I forgot to mention it in the patch I sent a few minutes ago. I'm not sure we
need to mention every error condition (specially one that will be rarely used).
(4) src/backend/replication/pgoutput/pgoutput.cpgoutput_change()The 3 added calls to pgoutput_row_filter() are returning frompgoutput_change(), if false is returned, but instead they should breakfrom the switch, otherwise cleanup code is missed. This is surely abug.
Fixed.
In summary, v18 contains
* Peter Smith's review
* Greg Nancarrow's review
* cache ExprState
* cache TupleTableSlot
* forbid custom operators
* various fixes
В списке pgsql-hackers по дате отправления:
Следующее
От: Ranier VilelaДата:
Сообщение: Protect against possible memory corruption (src/backend/access/nbtree/nbtxlog.c)