From: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | 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>, 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: | 2021-12-30 13:39:37 |
Message-ID: | OS0PR01MB57167079ADAF35476A30E49694459@OS0PR01MB5716.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Dec 29, 2021 11:16 AM Tang, Haiying <tanghy(dot)fnst(at)fujitsu(dot)com> wrote:
> On Mon, Dec 27, 2021 9:16 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> > On Thur, Dec 23, 2021 4:28 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > > Here is the v54* patch set:
> >
> > Attach the v55 patch set which add the following testcases in 0003 patch.
> > 1. Added a test to cover the case where TOASTed values are not included in the
> > new tuple. Suggested by Euler[1].
> >
> > Note: this test is temporarily commented because it would fail without
> > applying another bug fix patch in another thread[2] which log the
> detoasted
> > value in old value. I have verified locally that the test pass after
> > applying the bug fix patch[2].
> >
> > 2. Add a test to cover the case that transform the UPDATE into INSERT.
> Provided
> > by Tang.
> >
>
> Thanks for updating the patches.
>
> A few comments:
>
> 1) v55-0001
>
> -/*
> - * Gets the relations based on the publication partition option for a specified
> - * relation.
> - */
> List *
> GetPubPartitionOptionRelations(List *result, PublicationPartOpt pub_partopt,
> Oid relid)
>
> Do we need this change?
Added the comment back.
> 2) v55-0002
> * Multiple ExprState entries might be used if there are multiple
> * publications for a single table. Different publication actions don't
> * allow multiple expressions to always be combined into one, so there is
> - * one ExprSTate per publication action. Only 3 publication actions are
> + * one ExprState per publication action. Only 3 publication actions
> +are
> * used for row filtering ("insert", "update", "delete"). The exprstate
> * array is indexed by ReorderBufferChangeType.
> */
>
> I think this change can be merged into 0001 patch.
Merged.
> 3) v55-0002
> +static bool pgoutput_row_filter_update_check(enum
> ReorderBufferChangeType changetype, Relation relation,
> +
> HeapTuple oldtuple, HeapTuple newtuple,
> +
> RelationSyncEntry *entry, ReorderBufferChangeType *action);
>
> Do we need parameter changetype here? I think it could only be
> REORDER_BUFFER_CHANGE_UPDATE.
I didn't change this, I think it might be better to wait for Ajin's opinion.
Attach the v56 patch set which address above comments and comments(1. 2.) from [1]
Best regards,
Hou zj
Attachment | Content-Type | Size |
---|---|---|
v56-0001-Row-filter-for-logical-replication.patch | application/octet-stream | 131.9 KB |
v56-0004-refactor-row-filter-transformation.patch | application/octet-stream | 19.2 KB |
v56-0002-Row-filter-updates-based-on-old-new-tuples.patch | application/octet-stream | 34.0 KB |
v56-0003-Row-filter-tab-auto-complete-and-pgdump.patch | application/octet-stream | 5.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-12-30 16:24:55 | Re: Autovacuum and idle_session_timeout |
Previous Message | Guillaume Lelarge | 2021-12-30 13:18:42 | Re: Autovacuum and idle_session_timeout |