Re: row filtering for logical replication

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Önder Kalacı <onderkalaci(at)gmail(dot)com>, japin <japinli(at)hotmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, David Steele <david(at)pgmasters(dot)net>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: row filtering for logical replication
Date: 2022-01-06 22:44:25
Message-ID: CAHut+PvZLTuNemaNURamDcscisoZm=3mvOaBa3eKXk8Taq160Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 5, 2022 at 1:05 PM wangw(dot)fnst(at)fujitsu(dot)com
<wangw(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Thu, Jan 4, 2022 at 00:54 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > Modified in v58 [1] as suggested
> Thanks for updating the patches.
> A few comments about v58-0001 and v58-0002.
>
> v58-0001
> 1.
> How about modifying the following loop in copy_table by using for_each_from
> instead of foreach?
> Like the invocation of for_each_from in function get_rule_expr.
> from:
> if (qual != NIL)
> {
> ListCell *lc;
> bool first = true;
>
> appendStringInfoString(&cmd, " WHERE ");
> foreach(lc, qual)
> {
> char *q = strVal(lfirst(lc));
>
> if (first)
> first = false;
> else
> appendStringInfoString(&cmd, " OR ");
> appendStringInfoString(&cmd, q);
> }
> list_free_deep(qual);
> }
> change to:
> if (qual != NIL)
> {
> ListCell *lc;
> char *q = strVal(linitial(qual));
>
> appendStringInfo(&cmd, " WHERE %s", q);
> for_each_from(lc, qual, 1)
> {
> q = strVal(lfirst(lc));
> appendStringInfo(&cmd, " OR %s", q);
> }
> list_free_deep(qual);
> }
>

Modified as suggested in v59* [1]

> 2.
> I find the API of get_rel_sync_entry is modified.
> -get_rel_sync_entry(PGOutputData *data, Oid relid)
> +get_rel_sync_entry(PGOutputData *data, Relation relation)
> It looks like just moving the invocation of RelationGetRelid from outside into
> function get_rel_sync_entry. I am not sure whether this modification is
> necessary to this feature or not.
>

Fixed in v59* [1]. Removed the unnecessary changes.

> v58-0002
> 1.
> In function pgoutput_row_filter_init, if no_filter is set, I think we do not
> need to add row filter to list(rfnodes).
> So how about changing three conditions when add row filter to rfnodes like this:
> - if (pub->pubactions.pubinsert)
> + if (pub->pubactions.pubinsert && !no_filter[idx_ins])
> {
> rfnode = stringToNode(TextDatumGetCString(rfdatum));
> rfnodes[idx_ins] = lappend(rfnodes[idx_ins], rfnode);
> }
>

TODO.

------
[1] https://www.postgresql.org/message-id/CAHut%2BPsiw9fbOUTpCMWirut1ZD5hbWk8_U9tZya4mG-YK%2Bfq8g%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2022-01-06 22:45:51 Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
Previous Message Peter Smith 2022-01-06 22:39:14 Re: row filtering for logical replication