Re: row filtering for logical replication

On Tue, 02 Feb 2021 at 19:16, japin <japinli(at)hotmail(dot)com> wrote:
> On Tue, 02 Feb 2021 at 13:02, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> On Mon, Feb 01, 2021 at 04:11:50PM -0300, Euler Taveira wrote:
>>> After the commit 3696a600e2, the last patch does not apply cleanly. I'm
>>> attaching another version to address the documentation issues.
>>
>> I have bumped into this thread, and applied 0001. My guess is that
>> one of the patches developped originally for logical replication
>> defined atttypmod in LogicalRepRelation, but has finished by not using
>> it. Nice catch.
>
> Since the 0001 patch already be commited (4ad31bb2ef), we can remove it.

In 0003 patch, function GetPublicationRelationQuals() has been defined, but it
never used. So why should we define it?

$ grep 'GetPublicationRelationQuals' -rn src/
src/include/catalog/pg_publication.h:116:extern List *GetPublicationRelationQuals(Oid pubid);
src/backend/catalog/pg_publication.c:347:GetPublicationRelationQuals(Oid pubid)

If we must keep it, here are some comments on it.

(1)
value_datum = heap_getattr(tup, Anum_pg_publication_rel_prqual, RelationGetDescr(pubrelsrel), &isnull);

It looks too long, we can split it into two lines.

(2)
Since qual_value only used in "if (!isnull)" branch, so we can narrow it's scope.

(3)
Should we free the memory for qual_value?

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.