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 |
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" |