RE: Column Filtering in Logical Replication

2022-03-09 Thread houzj.f...@fujitsu.com
On Wednesday, March 9, 2022 6:04 PM Amit Kapila > On Mon, Mar 7, 2022 at 8:48 PM Tomas Vondra > wrote: > > > > On 3/4/22 11:42, Amit Kapila wrote: > > > > > * > > > Fetching column filter info in tablesync.c is quite expensive. It > > > seems to be using four round-trips to get the complete info

RE: Data is copied twice when specifying both child and parent table in publication

2022-03-09 Thread houzj.f...@fujitsu.com
Hi, When reviewing some logical replication related features. I noticed another possible problem if the subscriber subscribes multiple publications which publish parent and child table. For example: pub create table t (a int, b int, c int) partition by range (a); create table t_1 partition o

RE: Column Filtering in Logical Replication

2022-03-14 Thread houzj.f...@fujitsu.com
On Monday, March 14, 2022 5:08 AM Tomas Vondra wrote: > > On 3/12/22 05:30, Amit Kapila wrote: > >> ... > > > > Okay, please find attached. I have done basic testing of this, if we > > agree with this approach then this will require some more testing. > > > > Thanks, the proposed changes seem l

RE: logical replication empty transactions

2022-03-21 Thread houzj.f...@fujitsu.com
On Monday, March 21, 2022 6:01 PM Amit Kapila wrote: > > On Sat, Mar 19, 2022 at 9:10 AM Ajin Cherian wrote: > > > > On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila > wrote: > > > > > 3. Can we add a simple test for it in one of the existing test > > > files(say in 001_rep_changes.pl)? > > > > add

RE: logical replication empty transactions

2022-03-21 Thread houzj.f...@fujitsu.com
> On Monday, March 21, 2022 6:01 PM Amit Kapila > wrote: > > > > On Sat, Mar 19, 2022 at 9:10 AM Ajin Cherian wrote: > > > > > > On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila > > > > > wrote: > > > > > > > 3. Can we add a simple test for it in one of the existing test > > > > files(say in 001_rep

RE: logical replication empty transactions

2022-03-23 Thread houzj.f...@fujitsu.com
On Tuesday, March 22, 2022 7:50 PM Amit Kapila wrote: > On Tue, Mar 22, 2022 at 7:25 AM houzj.f...@fujitsu.com > wrote: > > > > > On Monday, March 21, 2022 6:01 PM Amit Kapila > > > > > > wrote: > > > > Oh, sorry, I posted the wrong patch, here

RE: Determine parallel-safety of partition relations for Inserts

2021-02-21 Thread houzj.f...@fujitsu.com
Hi, Attaching v7 patches with the changes: * rebase the code on the greg's latest parallel insert patch. Please consider it for further review. Best regards, houzj v7_0004-reloption-parallel_dml-test-and-doc.patch Description: v7_0004-reloption-parallel_dml-test-and-doc.patch v7_000

RE: Parallel INSERT (INTO ... SELECT ...)

2021-02-21 Thread houzj.f...@fujitsu.com
> Posting a new version of the patches, with the following updates: > - Moved the update of glob->relationOIDs (i.e. addition of partition OIDs that > plan depends on, resulting from parallel-safety checks) from within > max_parallel_hazard() to set_plan_references(). > - Added an extra test for pa

RE: A reloption for partitioned tables - parallel_workers

2021-02-22 Thread houzj.f...@fujitsu.com
> > 4. Maybe it also doesn't make sense to consider the parent relation's > > parallel_workers if Parallel Append is disabled > > (enable_parallel_append = off). That's because with a simple > > (non-parallel) Append running under Gather, all launched parallel > > workers process the same partitio

RE: A reloption for partitioned tables - parallel_workers

2021-02-23 Thread houzj.f...@fujitsu.com
> > It seems the patch does not include the code that get the > > parallel_workers from new struct " PartitionedTableRdOptions ", Did I miss > something ? > > Aren't the following hunks in the v2 patch what you meant? > > diff --git a/src/backend/access/common/reloptions.c > b/src/backend/access/

RE: Parallel INSERT (INTO ... SELECT ...)

2021-02-24 Thread houzj.f...@fujitsu.com
> > It is quite possible what you are saying is correct but I feel that is > > not this patch's fault. So, won't it better to discuss this in a > > separate thread? > > > > Good use case but again, I think this can be done as a separate patch. > > Agreed. > I think even the current patch offers gr

RE: Parallel INSERT (INTO ... SELECT ...)

2021-02-25 Thread houzj.f...@fujitsu.com
> I add some code to track the time spent in index operation. > From the results[1], we can see more workers will bring more cost in > _bt_search_insert() in each worker. > After debugged, the most cost part is the following: > - > /* drop the read lock on the page, then acquire o

RE: simplifying foreign key/RI checks

2021-03-01 Thread houzj.f...@fujitsu.com
Hi amit, (sorry about not cc the hacker list) I have an issue about command id here. It's probably not directly related to your patch, so I am sorry if it bothers you. + /* +* Start the scan. To make the changes of the current command visible to +* the scan and for subseque

Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-03 Thread houzj.f...@fujitsu.com
Hi hackers, Currently, postgres increments command id in ri trigger every time when inserting into a referencing table(fk relation). RI_FKey_check-> ri_PerformCheck->SPI_execute_snapshot-> _SPI_execute_plan-> CommandCounterIncrement It can be a block for supporting "parallel insert into a refer

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-03 Thread houzj.f...@fujitsu.com
Hi, > Why do we have to increment the command ID when the INSERT's target table > is a referenced table? Please refer to the explanation below. > > And INSERT command only have one target table, the modification on > > other tables can happen in the following cases. > > > > 1) has modifyingcte w

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-03 Thread houzj.f...@fujitsu.com
> From the wiki[1], CCI is to let statements can not see the rows they modify. Sorry, a typo here "not". I meant CCI is to let statements can see the rows they modify. Best regards, houzj

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-04 Thread houzj.f...@fujitsu.com
> > > I'm worried about having this dependency in RI check, because the > > > planner > > may allow parallel INSERT in these cases in the future. > > > > If we support parallel insert that can have other modifications in the > > future, I think we will also be able to share or increment command ID

RE: should INSERT SELECT use a BulkInsertState?

2021-03-08 Thread houzj.f...@fujitsu.com
> > I am very interested in this patch, and I plan to do some experiments with > > the > patch. > > Can you please rebase the patch because it seems can not applied to the > master now. > > Thanks for your interest. > > I was sitting on a rebased version since the bulk FDW patch will cause > co

RE: should INSERT SELECT use a BulkInsertState?

2021-03-08 Thread houzj.f...@fujitsu.com
> > > I am very interested in this patch, and I plan to do some > > > experiments with the > > patch. > > > Can you please rebase the patch because it seems can not applied to > > > the > > master now. > > > > Thanks for your interest. > > > > I was sitting on a rebased version since the bulk FDW p

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-08 Thread houzj.f...@fujitsu.com
> > > > I've attached an updated set of patches with the suggested locking changes. > > > > Amit L, others, do let me know if you have still more comments on > 0001* patch or if you want to review it further? I took a look into the latest 0001 patch, and it looks good to me. Best regards, houzj

Questions about CommandIsReadOnly

2021-03-09 Thread houzj.f...@fujitsu.com
Hi hackers, When reading the code, I found that in function CommandIsReadOnly[1], "select for update/share" is defined as "not read only". [1]- if (pstmt->rowMarks != NIL) return false; /* SELECT FOR [KEY] UPDATE/SHARE */ - And from t

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-09 Thread houzj.f...@fujitsu.com
> Attaching the first version patch which avoid CCI in RI trigger when insert > into > referencing table. After some more on how to support parallel insert into fk relation. It seems we do not have a cheap way to implement this feature. Please see the explanation below: In RI_FKey_check, Current

RE: Parallel Inserts in CREATE TABLE AS

2021-03-10 Thread houzj.f...@fujitsu.com
> Seems like v22 patch was failing in cfbot for one of the unstable test cases. > Attaching v23 patch set with modification in 0003 and 0004 patches. No > changes to 0001 and 0002 patches. Hopefully cfbot will be happy with v23. > > Please consider v23 for further review. Hi, I was looking into t

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-10 Thread houzj.f...@fujitsu.com
> > 2. Should we keep the default value of GUC to on or off? It is > > currently off. I am fine keeping it off for this release and we can > > always turn it on in the later releases if required. Having said that, > > I see the value in keeping it on because in many cases Insert ... > > Select will

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-11 Thread houzj.f...@fujitsu.com
> > After some more on how to support parallel insert into fk relation. > > It seems we do not have a cheap way to implement this feature. > > > > In RI_FKey_check, Currently, postgres execute "select xx for key > > share" to check that foreign key exists in PK table. > > However "select for updat

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-11 Thread houzj.f...@fujitsu.com
> I guess to have the finer granularity we'd have to go with > enable_parallel_insert, > which then would mean possibly having to later add enable_parallel_update, > should parallel update have similar potential overhead in the parallel-safety > checks (which to me, looks like it could, and parall

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-11 Thread houzj.f...@fujitsu.com
> On Thu, Mar 11, 2021 at 01:01:42PM +0000, houzj.f...@fujitsu.com wrote: > > > I guess to have the finer granularity we'd have to go with > > > enable_parallel_insert, which then would mean possibly having to > > > later add enable_parallel_update, should

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-12 Thread houzj.f...@fujitsu.com
> > The problem is that target_rel_trigger_max_parallel_hazard and its > > caller think they can use a relcache TriggerDesc field across other > > cache accesses, which they can't because the relcache doesn't > > guarantee that that won't move. > > > > One approach would be to add logic to Relation

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-12 Thread houzj.f...@fujitsu.com
> On Fri, Mar 12, 2021 at 6:10 AM Justin Pryzby > wrote: > > Note also this CF entry > > https://commitfest.postgresql.org/32/2987/ > > | Allow setting parallel_workers on partitioned tables > > +/* > + * PartitionedOptions > + * Contents of rd_options for partit

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-15 Thread houzj.f...@fujitsu.com
> > Attaching new version patch with this change. > > > > Thanks, the patch looks good to me. I have made some minor cosmetic > changes in the attached. I am planning to push this by tomorrow unless you or > others have any more comments or suggestions for this patch. Thanks for the review. I hav

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-16 Thread houzj.f...@fujitsu.com
> To support parallel insert into FK relation. > There are two scenarios need attention. > 1) foreign key and primary key are on the same table(INSERT's target table). > (referenced and referencing are the same table) > 2) referenced and referencing table are both partition of INSERT's target >

RE: Data is copied twice when specifying both child and parent table in publication

2021-12-02 Thread houzj.f...@fujitsu.com
On Thursday, December 2, 2021 12:50 PM Amit Kapila wrote: > On Thu, Dec 2, 2021 at 9:41 AM Greg Nancarrow > wrote: > > > > On Thu, Dec 2, 2021 at 1:48 PM Greg Nancarrow > wrote: > > > > > > If you updated my original description to say "(instead of just the > > > individual partitions)", it wou

RE: Data is copied twice when specifying both child and parent table in publication

2021-12-02 Thread houzj.f...@fujitsu.com
On Thursday, December 2, 2021 4:54 PM houzj.f...@fujitsu.com wrote: > On Thursday, December 2, 2021 12:50 PM Amit Kapila > wrote: > > On Thu, Dec 2, 2021 at 9:41 AM Greg Nancarrow > > wrote: > > > > > > On Thu, Dec 2, 2021 at 1:48 PM Greg Nancarrow > >

RE: parallel vacuum comments

2021-12-03 Thread houzj.f...@fujitsu.com
On Thur, Dec 2, 2021 8:31 PM Masahiko Sawada wrote: > I've attached updated patches. > > The first patch is the main patch for refactoring parallel vacuum code; > removes > bitmap-related code and renames function names for consistency. The second > patch moves these parallel-related codes to va

RE: pg_get_publication_tables() output duplicate relid

2021-12-05 Thread houzj.f...@fujitsu.com
On Thursday, December 2, 2021 9:48 PM Amit Langote wrote: > On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila wrote: > > On Thu, Dec 2, 2021 at 8:42 AM Amit Langote > wrote: > > > Reading Alvaro's email at the top again gave me a pause to > > > reconsider what I had said in reply. It might indeed hav

RE: parallel vacuum comments

2021-12-07 Thread houzj.f...@fujitsu.com
On Tuesday, December 7, 2021 1:42 PM Masahiko Sawada wrote: > I've attached an updated patch. I've removed 0003 patch that added > regression tests as per discussion. Regarding the terminology like "bulkdel" > and "cleanup" you pointed out, I've done that in 0002 patch while moving the > code to

RE: row filtering for logical replication

2021-12-08 Thread houzj.f...@fujitsu.com
On Wednesday, December 8, 2021 7:52 PM Ajin Cherian > On Tue, Dec 7, 2021 at 5:36 PM Peter Smith wrote: > > > > We were mid-way putting together the next v45* when your latest > > attachment was posted over the weekend. So we will proceed with our > > original plan to post our v45* (tomorrow). >

RE: parallel vacuum comments

2021-12-13 Thread houzj.f...@fujitsu.com
On Tuesday, December 14, 2021 10:11 AM Tang, Haiying wrote: > On Monday, December 13, 2021 2:12 PM Masahiko Sawada > wrote: > > I've attached the patch. I've just moved some functions back but not > > done other changes. > > > > Thanks for your patch. > > I tested your patch and tried some cases

RE: pg_get_publication_tables() output duplicate relid

2021-12-13 Thread houzj.f...@fujitsu.com
On Sat, Nov 20, 2021 7:31 PM Amit Kapila wrote: > On Fri, Nov 19, 2021 at 10:58 AM Amit Kapila > wrote: > > > > On Fri, Nov 19, 2021 at 7:19 AM Amit Langote > wrote: > > > > > > The problematic case is attaching the partition *after* the > > > subscriber has already marked the root parent as syn

RE: Failed transaction statistics to measure the logical replication progress

2021-12-14 Thread houzj.f...@fujitsu.com
On Tues, Dec 14, 2021 10:28 AM Amit Kapila wrote: > On Mon, Dec 13, 2021 at 5:48 PM osumi.takami...@fujitsu.com > wrote: > > > > On Monday, December 13, 2021 6:19 PM Amit Kapila > wrote: > > > On Tue, Dec 7, 2021 at 3:12 PM osumi.takami...@fujitsu.com > > > wrote: > > > > > > Few questions and

RE: parallel vacuum comments

2021-12-15 Thread houzj.f...@fujitsu.com
On Wed, Dec 15, 2021 4:03 PM Masahiko Sawada wrote: > On Tue, Dec 14, 2021 at 12:03 PM Amit Kapila wrote: > > There is still pending > > work related to moving parallel vacuum code to a separate file and a > > few other pending comments that are still under discussion. We can > > take care of tho

RE: row filtering for logical replication

2021-12-16 Thread houzj.f...@fujitsu.com
On Mon, Dec 13, 2021 5:49 PM Peter Smith wrote: > PSA the v46* patch set. > > Here are the main differences from v45: > 0. Rebased to HEAD > 1. Integrated many comments, docs, messages, code etc from Euler's patch > [Euler 6/12] 2. Several bugfixes 3. Patches are merged/added > > ~~ > > Bugfix

RE: Column Filtering in Logical Replication

2021-12-16 Thread houzj.f...@fujitsu.com
On Tues, Dec 14, 2021 1:48 AM Alvaro Herrera wrote: > Hmm, I messed up the patch file I sent. Here's the complete patch. > Hi, I have a minor question about the replica identity check of this patch. +check_publication_add_relation(Relation targetrel, Bitmapset *columns) ... +

RE: Column Filtering in Logical Replication

2021-12-17 Thread houzj.f...@fujitsu.com
On Friday, December 17, 2021 1:55 AM Alvaro Herrera wrote: > On 2021-Dec-16, houzj.f...@fujitsu.com wrote: > > > The patch ensures all columns of RT are in column list when > > CREATE/ALTER publication, but it seems doesn't prevent user from > > changing the rep

RE: row filtering for logical replication

2021-12-19 Thread houzj.f...@fujitsu.com
> -Original Message- > From: Amit Kapila On Saturday, December 18, 2021 10:33 AM > On Fri, Dec 17, 2021 at 5:29 PM Greg Nancarrow > wrote: > > > > On Fri, Dec 17, 2021 at 7:20 PM Ajin Cherian wrote: > > > > > > On Fri, Dec 17, 2021 at 5:46 PM Greg Nancarrow > wrote: > > > > > > > So u

relcache not invalidated when ADD PRIMARY KEY USING INDEX

2021-12-19 Thread houzj.f...@fujitsu.com
Hi, When reviewing some replica identity related patches, I found that when adding primary key using an existing unique index on not null columns, the target table's relcache won't be invalidated. This would cause an error When REPLICA IDENTITY is default and we are UPDATE/DELETE a published tabl

RE: parallel vacuum comments

2021-12-21 Thread houzj.f...@fujitsu.com
On Wed, Dec 22, 2021 8:38 AM Masahiko Sawada wrote: > On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila > wrote: > > > > On Tue, Dec 21, 2021 at 11:24 AM Masahiko Sawada > wrote: > > > > > > On Tue, Dec 21, 2021 at 2:04 PM Amit Kapila > wrote: > > > > > > > > > > Thank you for the comment. Agreed. >

RE: parallel vacuum comments

2021-12-22 Thread houzj.f...@fujitsu.com
On Wed, Dec 22, 2021 11:36 AM Masahiko Sawada wrote: > On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila > wrote: > > > > On Tue, Dec 21, 2021 at 11:24 AM Masahiko Sawada > wrote: > > > > > > On Tue, Dec 21, 2021 at 2:04 PM Amit Kapila > wrote: > > > > > > > > > > Thank you for the comment. Agreed.

Delay the variable initialization in get_rel_sync_entry

2021-12-22 Thread houzj.f...@fujitsu.com
Hi, When reviewing some logical replication patches. I noticed that in function get_rel_sync_entry() we always invoke get_rel_relispartition() and get_rel_relkind() at the beginning which could cause unnecessary cache access. --- get_rel_sync_entry(PGOutputData *data, Oid relid) { Relati

RE: parallel vacuum comments

2021-12-22 Thread houzj.f...@fujitsu.com
On Wed, Dec 22, 2021 9:55 PM Amit Kapila wrote: > On Wed, Dec 22, 2021 at 6:22 PM Amit Kapila > wrote: > > > > On Wed, Dec 22, 2021 at 5:39 PM houzj.f...@fujitsu.com > > wrote: > > > > > > > > > 2) > > > +#include "utils/

RE: Delay the variable initialization in get_rel_sync_entry

2021-12-24 Thread houzj.f...@fujitsu.com
On Friday, December 24, 2021 8:13 AM Michael Paquier wrote: > > On Thu, Dec 23, 2021 at 12:54:41PM -0300, Euler Taveira wrote: > > On Wed, Dec 22, 2021, at 10:11 AM, houzj.f...@fujitsu.com wrote: > >> The extra cost could sometimes be noticeable because get_rel_sync_

RE: row filtering for logical replication

2021-12-27 Thread houzj.f...@fujitsu.com
On Mon, Dec 27, 2021 9:16 PM houzj.f...@fujitsu.com wrote: > On Thur, Dec 23, 2021 4:28 PM Peter Smith wrote: > > Here is the v54* patch set: > > Attach the v55 patch set which add the following testcases in 0003 patch. Sorry for the typo here, I mean the tests are added 00

RE: Confused comment about drop replica identity index

2021-12-29 Thread houzj.f...@fujitsu.com
On Tues, Dec 21, 2021 8:47 AM Michael Paquier wrote: > On Mon, Dec 20, 2021 at 11:57:32AM -0300, Euler Taveira wrote: > > What do you think about the attached patch? It forbids the DROP INDEX. > > We might add a detail message but I didn't in this patch. > > Yeah. I'd agree about doing something

RE: Failed transaction statistics to measure the logical replication progress

2022-01-02 Thread houzj.f...@fujitsu.com
On Wednesday, December 22, 2021 6:14 PM Osumi, Takamichi wrote: > Attached the new patch v19. Hi, Thanks for updating the patch. --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -15,6 +15,7 @@ #include "portability/instr_time.h" #include "postmaster/pgarch.h" /* for MAX_XFN_CHARS */

RE: Delay the variable initialization in get_rel_sync_entry

2022-01-04 Thread houzj.f...@fujitsu.com
On Wednesday, January 5, 2022 9:31 AM Michael Paquier wrote: > On Fri, Dec 24, 2021 at 01:27:26PM +0000, houzj.f...@fujitsu.com wrote: > > Here is the perf result of pgoutput_change after applying the patch. > > I didn't notice something else that stand out. >

RE: row filtering for logical replication

2022-01-04 Thread houzj.f...@fujitsu.com
On Wednesday, January 5, 2022 2:45 PM Amit Kapila wrote: > > On Wed, Jan 5, 2022 at 11:04 AM Peter Smith > wrote: > > > > > > 11. src/backend/utils/cache/relcache.c - GetRelationPublicationActions > > > > Something seemed slightly fishy with the code doing the memcpy, > > because IIUC is possib

RE: row filtering for logical replication

2022-01-09 Thread houzj.f...@fujitsu.com
On Friday, January 7, 2022 3:40 PM Peter Smith wrote: > Below are some review comments for the v60 patches. > > 1e. > "Subsequent UPDATE or DELETE statements have no effect." > > Why won't they have an effect? The first impression is the newly updated tuple > now matches the filter, I think this

RE: row filtering for logical replication

2022-01-09 Thread houzj.f...@fujitsu.com
On Wednesday, January 5, 2022 2:01 PM vignesh C wrote: > On Tue, Jan 4, 2022 at 9:58 AM Peter Smith wrote: > > > > Here is the v58* patch set: > > > > Main changes from v57* are > > 1. Couple of review comments fixed > > > > ~~ > > > > Review comments (details) > > = > > >

RE: row filtering for logical replication

2022-01-13 Thread houzj.f...@fujitsu.com
On Thur, Jan 13, 2022 5:22 PM Alvaro Herrera wrote: > > /* > > + * Only 3 publication actions are used for row filtering ("insert", > > +"update", > > + * "delete"). See RelationSyncEntry.exprstate[]. > > + */ > > +typedef enum RowFilterPubAction > > +{ > > + PUBACTION_INSERT, > > + PUBACTION

RE: row filtering for logical replication

2022-01-13 Thread houzj.f...@fujitsu.com
On Thursday, January 13, 2022 2:49 PM Peter Smith > Thanks for posting the merged v63. > > Here are my review comments for the v63-0001 changes. > > ~~~ Thanks for the comments! > 1. src/backend/replication/logical/proto.c - logicalrep_write_tuple > > TupleDesc desc; > - Datum values[MaxTup

RE: Parallel Inserts in CREATE TABLE AS

2021-03-17 Thread houzj.f...@fujitsu.com
> > Seems like v22 patch was failing in cfbot for one of the unstable test > > cases. > > Attaching v23 patch set with modification in 0003 and 0004 patches. No > > changes to 0001 and 0002 patches. Hopefully cfbot will be happy with v23. > > > > Please consider v23 for further review. > Hi, > >

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-17 Thread houzj.f...@fujitsu.com
> > If a table parameter value is set and the > > equivalent toast. parameter is not, the TOAST table > > will use the table's parameter value. > > -Specifying these parameters for partitioned tables is not supported, > > -but you may specify them for individual leaf partitio

RE: Parallel Inserts in CREATE TABLE AS

2021-03-19 Thread houzj.f...@fujitsu.com
> They are pretty simple though. I think someone can also check if the same > regression exists for parallel inserts in "INSERT INTO SELECT" > patch set as well for larger tuple sizes. Thanks for reminding. I did some performance tests for parallel inserts in " INSERT INTO SELECT " with the testc

RE: Parallel Inserts in CREATE TABLE AS

2021-03-19 Thread houzj.f...@fujitsu.com
> > They are pretty simple though. I think someone can also check if the > > same regression exists for parallel inserts in "INSERT INTO SELECT" > > patch set as well for larger tuple sizes. > > Thanks for reminding. > I did some performance tests for parallel inserts in " INSERT INTO SELECT " >

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-21 Thread houzj.f...@fujitsu.com
> Since the "Parallel SELECT for INSERT" patch and related GUC/reloption patch > have been committed, I have now rebased and attached the rest of the original > patchset, > which includes: >- Additional tests for "Parallel SELECT for INSERT" >- Parallel INSERT functionality >- Test and documenta

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

2021-03-21 Thread houzj.f...@fujitsu.com
Hi, Thanks for the review. > I noticed some things on the first scan through: > > Patch 0001: > 1) Tidy up the comments a bit: > > Suggest the following update to part of the comments: Changed. > Patch 0002: > 1) The new max_parallel_hazard_context member "pk_rels" is not being > set (to > N

RE: Parallel INSERT (INTO ... SELECT ...)

2021-03-22 Thread houzj.f...@fujitsu.com
> > I noticed that some comments may need updated since we introduced > parallel insert in this patch. > > > > 1) src/backend/executor/execMain.c > > * Don't allow writes in parallel mode. Supporting UPDATE and > DELETE > > * would require (a) storing the combocid hash in shared

RE: should INSERT SELECT use a BulkInsertState?

2021-03-22 Thread houzj.f...@fujitsu.com
Hi, About the 0002-patch [Check for volatile defaults]. I wonder if we can check the volatile default value by traversing "query->targetList" in planner. IMO, the column default expression was written into the targetList, and the current parallel-safety check travere the "query->targetList" to

RE: [HACKERS] logical decoding of two-phase transactions

2021-03-24 Thread houzj.f...@fujitsu.com
> I have incorporated all your changes and additionally made few more changes > (a) got rid of LogicalRepBeginPrepareData and instead used > LogicalRepPreparedTxnData, (b) made a number of changes in comments and > docs, (c) ran pgindent, (d) modified tests to use standard wait_for_catch > function

RE: Add Nullif case for eval_const_expressions_mutator

2021-03-24 Thread houzj.f...@fujitsu.com
> + if (!has_nonconst_input) > + return ece_evaluate_expr(expr); > > That's not okay without a further check to see if the comparison function used > by the node is immutable. Compare ScalarArrayOpExpr, for instance. Thanks for poin

RE: fix typo in reorderbuffer.c

2021-03-24 Thread houzj.f...@fujitsu.com
> > What about "Combo CID(s)", for Combo command ID? README.parallel uses > this term for example. Sorry for the late reply and yes, " Combo CID(s)" looks better. Attaching patch which replaces all styles "Combocid(s)" with " Combo CID(s)". Best regards, houzj v2-0001-make-the-comments-about-

RE: Hybrid Hash/Nested Loop joins and caching results from subplans

2021-04-01 Thread houzj.f...@fujitsu.com
> I've attached the updated patch. I'll let the CFbot grab this to ensure it's > happy with it before I go looking to push it again. Hi, I took a look into the patch and noticed some minor things. 1. + case T_ResultCache: + ptype = "ResultCache"; +

RE: Table refer leak in logical replication

2021-04-05 Thread houzj.f...@fujitsu.com
> WARNING: relcache reference leak: relation "xxx" not closed. > > Example of the procedure: > --publisher-- > create table test (a int primary key); > create publication pub for table test; > > --subscriber-- > create table test (a int primary key); > create subscription sub conn

RE: Table refer leak in logical replication

2021-04-05 Thread houzj.f...@fujitsu.com
> > > insert into test values (6); > > > > > > It seems an issue about reference leak. Anyone can fix this? > > > > It seems ExecGetTriggerResultRel will reopen the target table because it > cannot find an existing one. > > Storing the opened table in estate->es_opened_result_relations seems > solv

RE: Added schema level support for publication.

2021-10-11 Thread houzj.f...@fujitsu.com
On Monday, October 11, 2021 2:39 PM vignesh C wrote: > > These comments are fixed in the v38 patch attached. Thanks for updating the patches. Here are a few comments on the v38-0004-Doc patch. 1. + + Adding/Setting a table that is part of schema specified in + ALL TABLES IN SCHEMA, adding

Drop replslot after pgstat_shutdown cause assert coredump

2021-10-11 Thread houzj.f...@fujitsu.com
Hi, When testing logical replication, I found a case which caused assert coredump on latest HEAD. The reproduction steps are as follows: 1) publisher create table test(i int); create publication pub for table test; begin; insert into test values(1); 2) subscriber create table te

RE: Added schema level support for publication.

2021-10-11 Thread houzj.f...@fujitsu.com
On Monday, October 11, 2021 11:02 PM vignesh C wrote: > The attached v39 patch has the fixes for the above issues. Thanks for the updates. I have a few minor suggestions about the testcases in the v39-0003-Test patch. 1) +-- alter publication drop CURRENT_SCHEMA +ALTER PUBLICATION testpub1_forsc

RE: Added schema level support for publication.

2021-10-12 Thread houzj.f...@fujitsu.com
On Tuesday, October 12, 2021 9:15 PM vignesh C > Attached v40 patch has the fix for the above comments. Thanks for the update, I have some minor issues about partition related behavior. 1) Tang tested and discussed this issue with me. The testcase is: We publish a schema and there is a partiti

RE: Failed transaction statistics to measure the logical replication progress

2021-10-13 Thread houzj.f...@fujitsu.com
On Thursday, September 30, 2021 12:15 PM Amit Kapila > On Thu, Sep 30, 2021 at 8:22 AM Hou, Zhijie/侯 志杰 > wrote: > > > > On Tues, Sep 28, 2021 6:05 PM Amit Kapila wrote: > > > > > > Can't we keep the current and new stats both in-memory and persist on > > > disk? So, the persistent stats data w

Data is copied twice when specifying both child and parent table in publication

2021-10-15 Thread houzj.f...@fujitsu.com
Hi, In another logical replication related thread[1], my colleague Greg found that if publish_via_partition_root is true, then the child table's data will be copied twice when adding both child and parent table to the publication. Example: - Pub: create table tbl1 (a int) partition by range

RE: Data is copied twice when specifying both child and parent table in publication

2021-10-15 Thread houzj.f...@fujitsu.com
On Friday, October 15, 2021 7:23 PM houzj.f...@fujitsu.com wrote: > Attach a patch to fix it. Attach a new version patch which refactor the fix code in a cleaner way. Best regards, Hou zj v2-0001-fix-double-publish.patch Description: v2-0001-fix-double-publish.patch

RE: Failed transaction statistics to measure the logical replication progress

2021-10-17 Thread houzj.f...@fujitsu.com
On Thursday, October 14, 2021 2:13 PM Osumi, Takamichi wrote: > On Thursday, October 14, 2021 12:54 PM Hou, Zhijie > wrote: > > On Thursday, September 30, 2021 12:15 PM Amit Kapila > > > On Thu, Sep 30, 2021 at 8:22 AM Hou, Zhijie/侯 志杰wrote: > > > > > > > > On Tues, Sep 28, 2021 6:05 PM Amit Kapil

RE: Added schema level support for publication.

2021-10-17 Thread houzj.f...@fujitsu.com
On Monday, October 18, 2021 12:04 PM Greg Nancarrow wrote: > On Sat, Oct 16, 2021 at 4:57 PM houzj.f...@fujitsu.com wrote: > > Based on the V40 patchset, attaching the Top-up patch which try to fix > > the partition issue in a cleaner way. > > > > A minor thing, in yo

RE: Data is copied twice when specifying both child and parent table in publication

2021-10-18 Thread houzj.f...@fujitsu.com
On Saturday, October 16, 2021 2:31 PM houzj.f...@fujitsu.com wrote: > On Friday, October 15, 2021 7:23 PM houzj.f...@fujitsu.com wrote: > > Attach a patch to fix it. > Attach a new version patch which refactor the fix code in a cleaner way. Although the discussion about the partitio

RE: Failed transaction statistics to measure the logical replication progress

2021-10-18 Thread houzj.f...@fujitsu.com
On Monday, October 18, 2021 6:04 PM Amit Kapila wrote: > On Thu, Oct 14, 2021 at 9:23 AM houzj.f...@fujitsu.com > wrote: > > > > On Thursday, September 30, 2021 12:15 PM Amit Kapila > > > > > > > > These all views are related to untransmitted to the co

RE: Data is copied twice when specifying both child and parent table in publication

2021-10-18 Thread houzj.f...@fujitsu.com
On Monday, October 18, 2021 5:03 PM Amit Langote wrote: > I can imagine that the behavior seen here may look surprising, but not > sure if I would call it a bug as such. I do remember thinking about > this case and the current behavior is how I may have coded it to be. > > Looking at this comma

RE: Skipping logical replication transactions on subscriber side

2021-10-19 Thread houzj.f...@fujitsu.com
On Mon, Oct 18, 2021 9:34 AM Masahiko Sawada wrote: > I've attached updated patches that incorporate all comments I got so far. Hi, Here are some minor comments for the patches. v17-0001-Add-a-subscription-errors-statistics-view-pg_sta.patch 1) + /* Clean up */ + if (not_ready_rel

RE: Added schema level support for publication.

2021-10-20 Thread houzj.f...@fujitsu.com
On Thurs, Oct 21, 2021 12:25 AM vignesh C wrote: > This version of patch retains the changes related to PublicationRelInfo, I > will > handle the merging of the patches in the next version so that this version of > patch change related to PublicationRelInfo can be reviewed easily. Thanks for the

RE: Skipping logical replication transactions on subscriber side

2021-10-26 Thread houzj.f...@fujitsu.com
On Thurs, Oct 21, 2021 12:59 PM Masahiko Sawada wrote: > I've attached updated patches. In this version, in addition to the review > comments I go so far, I've changed the view name from > pg_stat_subscription_errors to pg_stat_subscription_workers as per the > discussion on including xact info to

RE: Data is copied twice when specifying both child and parent table in publication

2021-10-28 Thread houzj.f...@fujitsu.com
Hi, As there are basically two separate issues mentioned in the thread, I tried to summarize the discussion so far which might be helpful to others. * The first issue[1]: If we include both the partitioned table and (explicitly) its child partitions in the publication when set publish_via_partit

RE: Added schema level support for publication.

2021-11-02 Thread houzj.f...@fujitsu.com
On Wed, Nov 3, 2021 12:25 PM vignesh C wrote: > On Wed, Nov 3, 2021 at 8:30 AM Amit Kapila > wrote: > > > > On Tue, Nov 2, 2021 at 8:13 PM Tomas Vondra > > wrote: > > > > > > > > > > > > > > Yeah, that is also true. So maybe at this, we can just rename the > > > > few types as suggested by you a

RE: Added schema level support for publication.

2021-11-03 Thread houzj.f...@fujitsu.com
On Thurs, Nov 4, 2021 8:12 AM Peter Smith wrote: > FYI - I found a small problem with one of the new PublicationObjSpec parser > error messages that was introduced by the recent schema publication commit > [1]. > > The error message text is assuming that the error originates from CREATE > PUBLICA

RE: row filtering for logical replication

2021-11-03 Thread houzj.f...@fujitsu.com
On Wednesday, November 3, 2021 8:51 PM Ajin Cherian wrote: > On Tue, Nov 2, 2021 at 10:44 PM Ajin Cherian wrote: > . > > > > The patch 0005 and 0006 has not yet been rebased but will be updated > > in a few days. > > > > Here's a rebase of all the 6 patches. Issue remaining: Thanks for the patc

RE: parallel vacuum comments

2021-11-04 Thread houzj.f...@fujitsu.com
On Thur, Nov 4, 2021 1:25 PM Masahiko Sawada wrote: > On Wed, Nov 3, 2021 at 1:08 PM Amit Kapila wrote: > > > > On Tue, Nov 2, 2021 at 11:17 AM Masahiko Sawada > > wrote: > > > > > > On Tue, Nov 2, 2021 at 5:57 AM Peter Geoghegan wrote: > > > > > > > > > > > Rather than inventing PARALLEL_VACUU

RE: Added schema level support for publication.

2021-11-05 Thread houzj.f...@fujitsu.com
> On Thu, Nov 4, 2021 at 3:24 PM vignesh C wrote: > > > > On Thu, Nov 4, 2021 at 5:41 AM Peter Smith > wrote: > > > > > > FYI - I found a small problem with one of the new PublicationObjSpec > > > parser error messages that was introduced by the recent schema > > > publication commit [1]. > > > >

RE: row filtering for logical replication

2021-11-07 Thread houzj.f...@fujitsu.com
On Fri, Nov 5, 2021 1:14 PM Peter Smith wrote: > PSA new set of v37* patches. Thanks for updating the patches. Few comments: 1) v37-0001 I think it might be better to also show the filter expression in '\d+ tablename' command after publication description. 2) v37-0004 + /* Scan the expr

RE: row filtering for logical replication

2021-11-09 Thread houzj.f...@fujitsu.com
On Fri, Nov 5, 2021 4:49 PM Amit Kapila wrote: > On Fri, Nov 5, 2021 at 10:44 AM Peter Smith wrote: > > > > PSA new set of v37* patches. > 3. > - | ColId > + | ColId OptWhereClause > { > $$ = makeNode(PublicationObjSpec); > $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION; > - $$->name = $1; >

RE: row filtering for logical replication

2021-11-09 Thread houzj.f...@fujitsu.com
On Thur, Nov 4, 2021 10:47 AM Peter Smith wrote: > PROPOSAL > > I propose that we change the way duplicate tables are processed to make it so > that it is always the *last* one that takes effect (instead of the *first* > one). AFAIK > doing this won't affect any current PG behaviour, but doing t

RE: row filtering for logical replication

2021-11-09 Thread houzj.f...@fujitsu.com
On Wed, Nov 10, 2021 10:48 AM Amit Kapila wrote: > On Tue, Nov 9, 2021 at 2:22 PM houzj.f...@fujitsu.com wrote: > > > > On Fri, Nov 5, 2021 4:49 PM Amit Kapila wrote: > > > On Fri, Nov 5, 2021 at 10:44 AM Peter Smith wrote: > > > > > > > > PSA n

RE: [BUG]Invalidate relcache when setting REPLICA IDENTITY

2021-11-10 Thread houzj.f...@fujitsu.com
On Wed, Nov 10, 2021 7:29 PM Amit Kapila wrote: > On Wed, Nov 10, 2021 at 12:42 PM tanghy.f...@fujitsu.com > wrote: > > > > Hi > > > > I think I found a bug related to logical replication(REPLICA IDENTITY in > specific). > > If I change REPLICA IDENTITY after creating publication, the > DELETE/U

  1   2   3   4   5   >