Re: row filtering for logical replication

On Tue, Dec 28, 2021 at 6:33 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Mon, Dec 27, 2021 9:19 PM Hou Zhijie <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > On Mon, Dec 27, 2021 9:16 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com>
> > wrote:
> > > On Thur, Dec 23, 2021 4:28 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > > > Here is the v54* patch set:
> > >
> > > Attach the v55 patch set which add the following testcases in 0002 patch.
>
> When reviewing the row filter patch, I found few things that could be improved.
> 1) We could transform the same row filter expression twice when
> ALTER PUBLICATION ... SET TABLE WHERE (...). Because we invoke
> GetTransformedWhereClause in both AlterPublicationTables() and
> publication_add_relation(). I was thinking it might be better if we only
> transform the expression once in AlterPublicationTables().
>
> 2) When transforming the expression, we didn’t set the correct p_sourcetext.
> Since we need to transforming serval expressions which belong to different
> relations, I think it might be better to pass queryString down to the actual
> transform function and set p_sourcetext to the actual queryString.
>

I have tried the following few examples to check the error_position
and it seems to be showing correct position without your 0004 patch.
postgres=# create publication pub for table t1 where (10);
ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer
LINE 1: create publication pub for table t1 where (10);

^

Also, transformPubWhereClauses() seems to be returning the same list
as it was passed to it. Do we really need to return anything from
transformPubWhereClauses()?

--
With Regards,
Amit Kapila.