Re: row filtering for logical replication

On Tue, Dec 28, 2021 at 6:33 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Mon, Dec 27, 2021 9:19 PM Hou Zhijie <houzj.fnst@fujitsu.com> wrote:
> > On Mon, Dec 27, 2021 9:16 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com>
> > wrote:
> > > On Thur, Dec 23, 2021 4:28 PM Peter Smith <smithpb2250@gmail.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.