Re: row filtering for logical replication

From: Ajin Cherian <itsajin(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(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 Kapila <amit(dot)kapila16(at)gmail(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: 2021-11-12 10:19:34
Message-ID: CAFPTHDZgxAbLA98HZkN-JNHoHXSBGtK4eAGQrXam5JQY6Zx7wg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attaching version 39-

V39 fixes the following review comments:

On Fri, Nov 5, 2021 at 7:49 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
> PUBLICATIONOBJ_TABLE);
>
>I think for the correct merge you need to just call
>CheckObjSchemaNotAlreadyInPublication() before this for loop. BTW, I
>have a question regarding this implementation. Here, it has been
>assumed that the new rel will always be specified with a different
>qual, what if there is no qual or if the qual is the same?

Actually with this code, no qual or a different qual does not matter,
it recreates everything as specified by the ALTER SET command.
I have added CheckObjSchemaNotAlreadyInPublication as you specified since this
is required to match the schema patch behaviour. I've also added
a test case that tests this particular case.

On Mon, Nov 8, 2021 at 5:53 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
>2) v37-0004
>
>+ /* Scan the expression tree for referenceable objects */
>+ find_expr_references_walker(expr, &context);
>+
>+ /* Remove any duplicates */
>+ eliminate_duplicate_dependencies(context.addrs);
>+
>
>The 0004 patch currently use find_expr_references_walker to get all the
>reference objects. I am thinking do we only need get the columns in the
>expression ? I think maybe we can check the replica indentity like[1].

Changed as suggested.

On Thu, Nov 4, 2021 at 2:08 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
>
>I see your point. But, I think we can add some comments here
>indicating that the user might have mistakenly given where clause with
>some schema which we will identify later and give an appropriate
>error. Then, in preprocess_pubobj_list(), identify if the user has
>given the where clause with schema name and give an appropriate erro

Changed as suggested.

On Thu, Nov 4, 2021 at 2:08 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
>BTW, why not give an error if the duplicate table is present and any one of them or
>both have row-filters? I think the current behavior makes sense
>because it makes no difference if the table is present more than once
>in the list but with row-filter it can make difference so it seems to
>me that giving an error should be considered.

Changed as suggested, also added test cases for the same.

regards,
Ajin Cherian
Fujitsu Australia

Attachment Content-Type Size
v39-0001-Row-filter-for-logical-replication.patch application/octet-stream 79.5 KB
v39-0003-PS-ExprState-cache-modifications.patch application/octet-stream 12.9 KB
v39-0004-PS-Row-filter-validation-of-replica-identity.patch application/octet-stream 22.1 KB
v39-0002-PS-Add-tab-auto-complete-support-for-the-Row-Fil.patch application/octet-stream 2.4 KB
v39-0005-PS-Row-filter-validation-walker.patch application/octet-stream 15.9 KB
v39-0006-Support-updates-based-on-old-and-new-tuple-in-ro.patch application/octet-stream 22.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2021-11-12 10:39:31 Re: Allow users to choose what happens when recovery target is not reached
Previous Message Bharath Rupireddy 2021-11-12 10:17:46 Re: add retry mechanism for achieving recovery target before emitting FATA error "recovery ended before configured recovery target was reached"