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

From Amit Kapila
Subject Re: row filtering for logical replication
Date
Msg-id CAA4eK1Jumuio6jZK8AVQd6z7gpDsZydQhK6d=MUARxk3nS7+Pw@mail.gmail.com
Whole thread Raw
In response to Re: row filtering for logical replication  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: row filtering for logical replication  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
List pgsql-hackers
On Mon, Jul 12, 2021 at 7:19 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Hi
>
> Andres complained about the safety of doing general expression
> evaluation in pgoutput; that was first in
>
> https://postgr.es/m/20210128022032.eq2qqc6zxkqn5syt@alap3.anarazel.de
> where he described a possible approach to handle it by restricting
> expressions to have limited shape; and later in
> http://postgr.es/m/20210331191710.kqbiwe73lur7jo2e@alap3.anarazel.de
>
> I was just scanning the patch trying to see if some sort of protection
> had been added for this, but I couldn't find anything.  (Some functions
> are under-commented, though).  So, is it there already, and if so what
> is it?
>

I think the patch is trying to prohibit arbitrary expressions in the
WHERE clause via
transformWhereClause(..EXPR_KIND_PUBLICATION_WHERE..). You can notice
that at various places the expressions are prohibited via
EXPR_KIND_PUBLICATION_WHERE. I am not sure that the checks are correct
and sufficient but I think there is some attempt to do it. For
example, the below sort of ad-hoc check for func_call doesn't seem to
be good idea.

@@ -119,6 +119,13 @@ transformExprRecurse(ParseState *pstate, Node *expr)
  /* Guard against stack overflow due to overly complex expressions */
  check_stack_depth();

+ /* Functions are not allowed in publication WHERE clauses */
+ if (pstate->p_expr_kind == EXPR_KIND_PUBLICATION_WHERE &&
nodeTag(expr) == T_FuncCall)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("functions are not allowed in publication WHERE expressions"),
+ parser_errposition(pstate, exprLocation(expr))));

Now, the other idea I had in mind was to traverse the WHERE clause
expression in publication_add_relation and identify if it contains
anything other than the ANDed list of 'foo.bar op constant'
expressions. OTOH, for index where clause expressions or policy check
expressions, we use a technique similar to what we have in the patch
to prohibit certain kinds of expressions.

Do you have any preference on how this should be addressed?

-- 
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Remove useless int64 range checks on BIGINT sequence MINVALUE/MAXVALUE values
Next
From: Greg Nancarrow
Date:
Subject: Re: Remove useless int64 range checks on BIGINT sequence MINVALUE/MAXVALUE values