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

RE: Determine parallel-safety of partition relations for Inserts

2021-02-17 Thread Hou, Zhijie
Hi, Attaching v6 patches with the changes: * rebase the code on the greg's latest parallel insert patch. * fix some code comment. * add some test to cover the partitioned table. Please consider it for further review. Best regards, Houzj

RE: Determine parallel-safety of partition relations for Inserts

2021-02-04 Thread Huang, Qiuyan
d is set to off Regards Huang > -Original Message- > From: Hou, Zhijie > Sent: Wednesday, February 3, 2021 9:01 AM > To: Greg Nancarrow > Cc: Amit Kapila ; PostgreSQL Hackers > ; vignesh C ; > Amit Langote ; David Rowley > ; Tom Lane ; Tsunakawa, > Takayuki/綱川

RE: Determine parallel-safety of partition relations for Inserts

2021-02-02 Thread Hou, Zhijie
> For v3_0003-reloption-parallel_dml-src.patch : > +   table_close(rel, NoLock); > Since the rel would always be closed, it seems the return value from  > RelationGetParallelDML() can be assigned to a variable, followed by call to  > table_close(), then the return statement. Thanks for the

RE: Determine parallel-safety of partition relations for Inserts

2021-02-02 Thread Hou, Zhijie
Hi, Attaching v5 patches with the changes: * rebase the code on the greg's latest parallel insert patch * fix some code style. Please consider it for further review. Best regards, Houzj v5_0004-reloption-parallel_dml-test-and-doc.patch Description:

RE: Determine parallel-safety of partition relations for Inserts

2021-02-01 Thread Hou, Zhijie
> > IMO, It seems more readable to extract all the check that we can do > > before the safety-check and put them in the new function. > > > > Please consider it for further review. > > > > I've updated your v2 patches and altered some comments and documentation > changes (but made no code

Re: Determine parallel-safety of partition relations for Inserts

2021-02-01 Thread Zhihong Yu
Hi, For v3_0003-reloption-parallel_dml-src.patch : +* Check if parallel_dml_enabled is enabled for the target table, +* if not, skip the safety checks and return PARALLEL_UNSAFE. Looks like the return value is true / false. So the above comment should be adjusted. + if

Re: Determine parallel-safety of partition relations for Inserts

2021-02-01 Thread Greg Nancarrow
On Mon, Feb 1, 2021 at 4:02 PM Hou, Zhijie wrote: > > Attatching v2 patch which addressed the comments above. > > Some further refactor: > > Introducing a new function is_parallel_possible_for_modify() which decide > whether to do safety check. > > IMO, It seems more readable to extract all the

RE: Determine parallel-safety of partition relations for Inserts

2021-01-31 Thread Hou, Zhijie
Hi greg, Thanks for the review ! > Personally, I think a table's "parallel_dml" option should be ON by default. > It's annoying to have to separately enable it for each and every table being > used, when I think the need to turn it selectively OFF should be fairly > rare. Yes, I agreed.

Re: Determine parallel-safety of partition relations for Inserts

2021-01-31 Thread Greg Nancarrow
On Fri, Jan 29, 2021 at 5:44 PM Hou, Zhijie wrote: > > > Attatching v1 patches, introducing options which let user manually control > whether to use parallel dml. > > About the patch: > 1. add a new guc option: enable_parallel_dml (boolean) > 2. add a new tableoption: parallel_dml (boolean) > >

RE: Determine parallel-safety of partition relations for Inserts

2021-01-28 Thread Hou, Zhijie
> Hi, > > Attatching v1 patches, introducing options which let user manually control > whether to use parallel dml. > > About the patch: > 1. add a new guc option: enable_parallel_dml (boolean) 2. add a new > tableoption: parallel_dml (boolean) > >The default of both is off(false). > >

RE: Determine parallel-safety of partition relations for Inserts

2021-01-28 Thread Hou, Zhijie
Hi, Attatching v1 patches, introducing options which let user manually control whether to use parallel dml. About the patch: 1. add a new guc option: enable_parallel_dml (boolean) 2. add a new tableoption: parallel_dml (boolean) The default of both is off(false). User can set

RE: Determine parallel-safety of partition relations for Inserts

2021-01-28 Thread Hou, Zhijie
> > Hi > > > > I have an issue about the parameter for DML. > > > > If we define the parameter as a tableoption. > > > > For a partitioned table, if we set the parent table's parallel_dml=on, > > and set one of its partition parallel_dml=off, it seems we can not divide > the plan for the separate

Re: Determine parallel-safety of partition relations for Inserts

2021-01-28 Thread Amit Kapila
On Thu, Jan 28, 2021 at 5:00 PM Hou, Zhijie wrote: > > > From: Amit Kapila > > > Good question. I think if we choose to have a separate parameter for > > > DML, it can probably a boolean to just indicate whether to enable > > > parallel DML for a specified table and use the parallel_workers > >

RE: Determine parallel-safety of partition relations for Inserts

2021-01-28 Thread Hou, Zhijie
> From: Amit Kapila > > Good question. I think if we choose to have a separate parameter for > > DML, it can probably a boolean to just indicate whether to enable > > parallel DML for a specified table and use the parallel_workers > > specified in the table used in SELECT. > > Agreed. Hi I

RE: Determine parallel-safety of partition relations for Inserts

2021-01-18 Thread tsunakawa.ta...@fujitsu.com
From: Amit Kapila > Good question. I think if we choose to have a separate parameter for > DML, it can probably a boolean to just indicate whether to enable > parallel DML for a specified table and use the parallel_workers > specified in the table used in SELECT. Agreed. Regards Takayuki

Re: Determine parallel-safety of partition relations for Inserts

2021-01-18 Thread Amit Kapila
On Mon, Jan 18, 2021 at 10:27 AM tsunakawa.ta...@fujitsu.com wrote: > > From: Amit Kapila > > We already allow users to specify the degree of parallelism for all > > the parallel operations via guc's max_parallel_maintenance_workers, > > max_parallel_workers_per_gather, then we have a reloption

RE: Determine parallel-safety of partition relations for Inserts

2021-01-17 Thread tsunakawa.ta...@fujitsu.com
From: Amit Kapila > We already allow users to specify the degree of parallelism for all > the parallel operations via guc's max_parallel_maintenance_workers, > max_parallel_workers_per_gather, then we have a reloption > parallel_workers and vacuum command has the parallel option where > users can

Re: Determine parallel-safety of partition relations for Inserts

2021-01-17 Thread Amit Kapila
On Mon, Jan 18, 2021 at 6:08 AM tsunakawa.ta...@fujitsu.com wrote: > > From: Amit Kapila > > I think it would be good if the parallelism works by default when > > required but I guess if we want to use something on these lines then > > we can always check if the parallel_workers option is

Re: Determine parallel-safety of partition relations for Inserts

2021-01-17 Thread Amit Kapila
On Sun, Jan 17, 2021 at 4:45 PM Amit Langote wrote: > > On Sat, Jan 16, 2021 at 2:02 PM Amit Kapila wrote: > > On Fri, Jan 15, 2021 at 6:45 PM Amit Langote > > wrote: > > > On Fri, Jan 15, 2021 at 9:59 PM Amit Kapila > > > wrote: > > > > We want to do this for Inserts where only Select can

RE: Determine parallel-safety of partition relations for Inserts

2021-01-17 Thread tsunakawa.ta...@fujitsu.com
From: Amit Kapila > I think it would be good if the parallelism works by default when > required but I guess if we want to use something on these lines then > we can always check if the parallel_workers option is non-zero for a > relation (with RelationGetParallelWorkers). So users can always say

Re: Determine parallel-safety of partition relations for Inserts

2021-01-17 Thread Amit Langote
On Sat, Jan 16, 2021 at 2:02 PM Amit Kapila wrote: > On Fri, Jan 15, 2021 at 6:45 PM Amit Langote wrote: > > On Fri, Jan 15, 2021 at 9:59 PM Amit Kapila wrote: > > > We want to do this for Inserts where only Select can be parallel and > > > Inserts will always be done by the leader backend.

Re: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Amit Kapila
On Fri, Jan 15, 2021 at 7:35 PM tsunakawa.ta...@fujitsu.com wrote: > > From: Amit Kapila > > This will surely increase planning time but the execution is reduced > > to an extent due to parallelism that it won't matter for either of the > > cases if we see just total time. For example, see the

Re: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Amit Kapila
On Fri, Jan 15, 2021 at 6:45 PM Amit Langote wrote: > > On Fri, Jan 15, 2021 at 9:59 PM Amit Kapila wrote: > > We want to do this for Inserts where only Select can be parallel and > > Inserts will always be done by the leader backend. This is actually > > the case we first want to implement. > >

RE: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread tsunakawa.ta...@fujitsu.com
From: Amit Langote > Sorry, I haven't looked at the linked threads and the latest patches > there closely enough yet, so I may be misreading this, but if the > inserts will always be done by the leader backend as you say, then why > does the planner need to be checking the parallel safety of the

RE: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread tsunakawa.ta...@fujitsu.com
From: Amit Kapila > This will surely increase planning time but the execution is reduced > to an extent due to parallelism that it won't matter for either of the > cases if we see just total time. For example, see the latest results > for parallel inserts posted by Haiying Tang [3]. There might

Re: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Amit Langote
On Fri, Jan 15, 2021 at 9:59 PM Amit Kapila wrote: > We want to do this for Inserts where only Select can be parallel and > Inserts will always be done by the leader backend. This is actually > the case we first want to implement. Sorry, I haven't looked at the linked threads and the latest

Re: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Amit Kapila
On Fri, Jan 15, 2021 at 5:53 PM Ashutosh Bapat wrote: > > On Fri, Jan 15, 2021 at 3:48 PM Amit Kapila wrote: > > > > While reviewing parallel insert [1] (Insert into Select) and > > parallel copy patches [2], it came to my notice that both the patches > > traverse the entire partition

Re: Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Ashutosh Bapat
On Fri, Jan 15, 2021 at 3:48 PM Amit Kapila wrote: > > While reviewing parallel insert [1] (Insert into Select) and > parallel copy patches [2], it came to my notice that both the patches > traverse the entire partition hierarchy to determine parallel-safety > of partitioned relations. This

Determine parallel-safety of partition relations for Inserts

2021-01-15 Thread Amit Kapila
While reviewing parallel insert [1] (Insert into Select) and parallel copy patches [2], it came to my notice that both the patches traverse the entire partition hierarchy to determine parallel-safety of partitioned relations. This is required because before considering the Insert or Copy can