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

On Mon, Mar 22, 2021 at 3:57 PM Greg Nancarrow wrote: > > On Mon, Mar 22, 2021 at 6:28 PM houzj.f...@fujitsu.com > wrote: > > > > > > > > Let me know if these changes seem OK to you. > > > > Yes, these changes look good to me. > > Posting an updated set of patches with these changes... > I have

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

> > 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: Parallel INSERT (INTO ... SELECT ...)

On Mon, Mar 22, 2021 at 2:30 PM houzj.f...@fujitsu.com wrote: > > 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 > *

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

> 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: Parallel INSERT (INTO ... SELECT ...)

On Thu, Mar 18, 2021 at 9:04 AM houzj.f...@fujitsu.com wrote: > > > > 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

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

> > 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 INSERT (INTO ... SELECT ...)

On Thu, Mar 18, 2021 at 8:30 AM Justin Pryzby wrote: > > diff --git a/doc/src/sgml/ref/create_table.sgml > b/doc/src/sgml/ref/create_table.sgml > index ff1b642722..d5d356f2de 100644 > --- a/doc/src/sgml/ref/create_table.sgml > +++ b/doc/src/sgml/ref/create_table.sgml > @@ -1338,8 +1338,10 @@ WITH

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

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index ff1b642722..d5d356f2de 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -1338,8 +1338,10 @@ WITH ( MODULUS numeric_literal, REM If a table parameter value is s

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

On Fri, Mar 12, 2021 at 04:05:09PM +0900, Amit Langote wrote: > 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 I'm rebasing that other patch on top of master

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

> > 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: Parallel INSERT (INTO ... SELECT ...)

On Mon, Mar 15, 2021 at 11:34 AM Justin Pryzby wrote: > > On Mon, Mar 15, 2021 at 11:25:26AM +0530, Amit Kapila wrote: > > On Fri, Mar 12, 2021 at 3:01 PM houzj.f...@fujitsu.com wrote: > > > > > > Attaching new version patch with this change. > > > > Thanks, the patch looks good to me. I have made

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

On Mon, Mar 15, 2021 at 11:25:26AM +0530, Amit Kapila wrote: > On Fri, Mar 12, 2021 at 3:01 PM houzj.f...@fujitsu.com wrote: > > > > 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 pus

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

On Fri, Mar 12, 2021 at 3:01 PM houzj.f...@fujitsu.com wrote: > > 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 sugg

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

On Sat, Mar 13, 2021 at 10:08 AM Tom Lane wrote: > > Greg Nancarrow writes: > > On Fri, Mar 12, 2021 at 5:00 AM Tom Lane wrote: > >> BTW, having special logic for FK triggers in > >> target_rel_trigger_max_parallel_hazard seems quite loony to me. > >> Why isn't that handled by setting appropriat

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

Greg Nancarrow writes: > On Fri, Mar 12, 2021 at 5:00 AM Tom Lane wrote: >> BTW, having special logic for FK triggers in >> target_rel_trigger_max_parallel_hazard seems quite loony to me. >> Why isn't that handled by setting appropriate proparallel values >> for those trigger functions? > ... an

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

On Fri, Mar 12, 2021 at 9:33 AM houzj.f...@fujitsu.com wrote: > > > On Thu, Mar 11, 2021 at 01:01:42PM +, 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

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

On Fri, Mar 12, 2021 at 1:33 PM houzj.f...@fujitsu.com wrote: > > > > 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 tha

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

> 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 ...)

> > 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 ...)

On Fri, Mar 12, 2021 at 5:00 AM Tom Lane wrote: > > > 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 a

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

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 partitioned tables + */ +typedef struct PartitionedOpt

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

> On Thu, Mar 11, 2021 at 01:01:42PM +, 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 parallel update have > > > similar potenti

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

On Fri, Mar 12, 2021 at 5:00 AM Tom Lane wrote: > > The buildfarm has revealed that this patch doesn't work under > CLOBBER_CACHE_ALWAYS: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=husky&dt=2021-03-10%2021%3A09%3A32 > > I initially thought that that was a problem with c3ffe34863,

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

On Thu, Mar 11, 2021 at 01:01:42PM +, 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 parallel update have similar potential overhe

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

The buildfarm has revealed that this patch doesn't work under CLOBBER_CACHE_ALWAYS: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=husky&dt=2021-03-10%2021%3A09%3A32 I initially thought that that was a problem with c3ffe34863, but after reproducing it here I get this stack trace: #0 ta

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

> 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 ...)

On Wed, Mar 10, 2021 at 8:18 PM Amit Kapila wrote: > > Now, coming back to Hou-San's patch to introduce a GUC and reloption > for this feature, I think both of those make sense to me because when > the feature is enabled via GUC, one might want to disable it for > partitioned tables? Do we agree o

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

On Wed, Mar 10, 2021 at 6:18 PM Amit Kapila wrote: > On Mon, Mar 8, 2021 at 7:19 PM Amit Langote wrote: > > I just read through v25 and didn't find anything to complain about. > > Thanks a lot, pushed now! Amit L., your inputs are valuable for this work. Glad I could be of help. Really apprecia

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

From: Amit Kapila > Now, coming back to Hou-San's patch to introduce a GUC and reloption > for this feature, I think both of those make sense to me because when > the feature is enabled via GUC, one might want to disable it for > partitioned tables? Do we agree on that part or someone thinks > oth

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

> > 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: Parallel INSERT (INTO ... SELECT ...)

On Wed, Mar 10, 2021 at 2:48 PM Amit Kapila wrote: > > On Mon, Mar 8, 2021 at 7:19 PM Amit Langote wrote: > > > > Hi Amit > > > > On Mon, Mar 8, 2021 at 10:18 PM Amit Kapila wrote: > > > On Mon, Mar 8, 2021 at 3:54 PM Greg Nancarrow wrote: > > > > I've attached an updated set of patches with th

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

On Mon, Mar 8, 2021 at 7:19 PM Amit Langote wrote: > > Hi Amit > > On Mon, Mar 8, 2021 at 10:18 PM Amit Kapila wrote: > > On Mon, Mar 8, 2021 at 3:54 PM Greg Nancarrow wrote: > > > I've attached an updated set of patches with the suggested locking > > > changes. > > (Thanks Greg.) > > > Amit L,

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

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

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

Hi Amit On Mon, Mar 8, 2021 at 10:18 PM Amit Kapila wrote: > On Mon, Mar 8, 2021 at 3:54 PM Greg Nancarrow wrote: > > I've attached an updated set of patches with the suggested locking changes. (Thanks Greg.) > Amit L, others, do let me know if you have still more comments on > 0001* patch or

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

On Mon, Mar 8, 2021 at 3:54 PM Greg Nancarrow wrote: > > 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? -- With Regards, Amit Kapila.

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

On Mon, Mar 8, 2021 at 6:25 PM Amit Langote wrote: > > A couple of things that look odd in v24-0001 (sorry if there were like > that from the beginning): > > +static bool > +target_rel_max_parallel_hazard(max_parallel_hazard_context *context) > +{ > + boolmax_hazard_found; > + > + Rela

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

On Mon, Mar 8, 2021 at 12:55 PM Amit Langote wrote: > > Hi Amit, Greg, > > Sorry, I hadn't noticed last week that some questions were addressed to me. > > On Sat, Mar 6, 2021 at 7:19 PM Amit Kapila wrote: > > Thanks, your changes look good to me. I went ahead and changed the > > patch to track th

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

Hi Amit, Greg, Sorry, I hadn't noticed last week that some questions were addressed to me. On Sat, Mar 6, 2021 at 7:19 PM Amit Kapila wrote: > Thanks, your changes look good to me. I went ahead and changed the > patch to track the partitionOids once rather than in two separate > lists and use th

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

From: Amit Kapila > For now, I have left 0005 and 0006 patches, we can come back to those once we > are done with the first set of patches. The first patch looks good to me and I > think we can commit it and then bikeshed about GUC/reloption patch. Agreed, it looks good to me, too. Regards Taka

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

On Sat, Mar 6, 2021 at 9:19 PM Amit Kapila wrote: > > On Fri, Mar 5, 2021 at 6:34 PM Greg Nancarrow wrote: > > > > For the time being at least, I am posting an updated set of patches, > > as I found that the additional parallel-safety checks on DOMAIN check > > constraints to be somewhat ineffici

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

For cfffe83ba82021a1819a656e7ec5c28fb3a99152, if a bool was written (true | false), READ_INT_FIELD calls atoi() where atoi("true") returns 0 and atoi("false") returns 0 as well. I am not sure if the new release containing these commits had a higher cat version compared to the previous release. If

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

On Sun, Mar 7, 2021 at 8:24 AM Zhihong Yu wrote: > > I was looking at src/backend/nodes/readfuncs.c > > READ_NODE_FIELD(relationOids); > + READ_NODE_FIELD(partitionOids); > > READ_NODE_FIELD would call nodeRead() for partitionOids. However, such field > may not exist. > Since there is no 'i

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

I was looking at src/backend/nodes/readfuncs.c READ_NODE_FIELD(relationOids); + READ_NODE_FIELD(partitionOids); READ_NODE_FIELD would call nodeRead() for partitionOids. However, such field may not exist. Since there is no 'if (strncmp(":partitionOids", token, length) == 0) {' check, I was c

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

On Sat, Mar 6, 2021 at 9:13 PM Zhihong Yu wrote: > > Hi, > Does CATALOG_VERSION_NO need to be bumped (introduction of partitionOids > field) ? > Good question. I usually update CATALOG_VERSION_NO when the patch changes any of the system catalogs. This is what is also mentioned in catversion.h. S

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

Hi, Does CATALOG_VERSION_NO need to be bumped (introduction of partitionOids field) ? Cheers On Sat, Mar 6, 2021 at 2:19 AM Amit Kapila wrote: > On Fri, Mar 5, 2021 at 6:34 PM Greg Nancarrow wrote: > > > > For the time being at least, I am posting an updated set of patches, > > as I found that

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

On Fri, Mar 5, 2021 at 6:34 PM Greg Nancarrow wrote: > > On Fri, Mar 5, 2021 at 9:35 PM Amit Kapila wrote: > > > > On Fri, Mar 5, 2021 at 8:24 AM Greg Nancarrow wrote: > > > > > > > In patch v21-0003-Add-new-parallel-dml-GUC-and-table-options, we are > > introducing GUC (enable_parallel_dml) and

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

On Fri, Mar 5, 2021 at 8:24 AM Greg Nancarrow wrote: > In patch v21-0003-Add-new-parallel-dml-GUC-and-table-options, we are introducing GUC (enable_parallel_dml) and table option (parallel_dml_enabled) for this feature. I am a bit worried about using *_dml in the names because it is quite possibl

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

On Fri, Mar 5, 2021 at 5:07 AM Greg Nancarrow wrote: > > On Thu, Mar 4, 2021 at 11:05 PM Amit Kapila wrote: > > > > On Thu, Mar 4, 2021 at 5:24 PM Greg Nancarrow wrote: > > > > > > On Thu, Mar 4, 2021 at 10:07 PM Amit Kapila > > > wrote: > > > > > > > > > > >Also, in > > > > standard_planner,

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

On Thu, Mar 4, 2021 at 11:05 PM Amit Kapila wrote: > > On Thu, Mar 4, 2021 at 5:24 PM Greg Nancarrow wrote: > > > > On Thu, Mar 4, 2021 at 10:07 PM Amit Kapila wrote: > > > > > > > >Also, in > > > standard_planner, we should add these partitionOids only for > > > parallel-mode. > > > > > > > It

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

On Thu, Mar 4, 2021 at 5:24 PM Greg Nancarrow wrote: > > On Thu, Mar 4, 2021 at 10:07 PM Amit Kapila wrote: > > > > >Also, in > > standard_planner, we should add these partitionOids only for > > parallel-mode. > > > > It is doing that in v20 patch (what makes you think it isn't?). > The below co

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

On Thu, Mar 4, 2021 at 10:07 PM Amit Kapila wrote: > > On Fri, Feb 26, 2021 at 10:37 AM Amit Langote wrote: > > > > I realized that there is a race condition that will allow a concurrent > > backend to invalidate a parallel plan (for insert into a partitioned > > table) at a point in time when it

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

On Thu, Mar 4, 2021 at 4:37 PM Amit Kapila wrote: > > On Fri, Feb 26, 2021 at 10:37 AM Amit Langote wrote: > > > > I realized that there is a race condition that will allow a concurrent > > backend to invalidate a parallel plan (for insert into a partitioned > > table) at a point in time when it'

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

On Fri, Feb 26, 2021 at 10:37 AM Amit Langote wrote: > > I realized that there is a race condition that will allow a concurrent > backend to invalidate a parallel plan (for insert into a partitioned > table) at a point in time when it's too late for plancache.c to detect > it. It has to do with h

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

On Thu, Mar 4, 2021 at 9:03 AM Amit Kapila wrote: > > I think for Update/Delete, we might not do parallel-safety checks by > calling target_rel_max_parallel_hazard_recurse especially because > partitions are handled differently for Updates and Deletes (see > inheritance_planner()). I think what Di

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

On Thu, Mar 4, 2021 at 7:16 AM Greg Nancarrow wrote: > > On Thu, Mar 4, 2021 at 1:07 AM Dilip Kumar wrote: > > > > On Wed, Mar 3, 2021 at 5:50 PM Greg Nancarrow wrote: > > > > > I agree that assert is only for debug build, but once we add and > > assert that means we are sure that it should only

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

On Thu, Mar 4, 2021 at 7:16 AM Greg Nancarrow wrote: > > On Thu, Mar 4, 2021 at 1:07 AM Dilip Kumar wrote: > > > > On Wed, Mar 3, 2021 at 5:50 PM Greg Nancarrow wrote: > > > > > > Asserts are normally only enabled in a debug-build, so for a > > > release-build that Assert has no effect. > > > Th

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

On Thu, Mar 4, 2021 at 1:07 AM Dilip Kumar wrote: > > On Wed, Mar 3, 2021 at 5:50 PM Greg Nancarrow wrote: > > > > Asserts are normally only enabled in a debug-build, so for a > > release-build that Assert has no effect. > > The Assert is being used as a sanity-check that the function is only > >

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

On Wed, Mar 3, 2021 at 5:50 PM Greg Nancarrow wrote: > > Asserts are normally only enabled in a debug-build, so for a > release-build that Assert has no effect. > The Assert is being used as a sanity-check that the function is only > currently getting called for INSERT, because that's all it curre

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

On Wed, Mar 3, 2021 at 10:45 PM Dilip Kumar wrote: > > On Mon, Mar 1, 2021 at 9:08 AM Greg Nancarrow wrote: > > > > Posting an updated set of patches that includes Amit Langote's patch > > to the partition tracking scheme... > > (the alternative of adding partitions to the range table needs furth

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

On Mon, Mar 1, 2021 at 9:08 AM Greg Nancarrow wrote: > > Posting an updated set of patches that includes Amit Langote's patch > to the partition tracking scheme... > (the alternative of adding partitions to the range table needs further > investigation) I was reviewing your latest patch and I hav

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

On Wed, Mar 3, 2021 at 12:52 PM Greg Nancarrow wrote: > > On Tue, Mar 2, 2021 at 11:19 PM Amit Kapila wrote: > > > > On Mon, Mar 1, 2021 at 9:08 AM Greg Nancarrow wrote: > > > 2. > > /* > > + * Prepare for entering parallel mode by assigning a > > + * FullTransactionId, to be included in the tra

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

On Tue, Mar 2, 2021 at 11:19 PM Amit Kapila wrote: > > On Mon, Mar 1, 2021 at 9:08 AM Greg Nancarrow wrote: > > > > Posting an updated set of patches that includes Amit Langote's patch > > to the partition tracking scheme... > > > > Few comments: > == > 1. > "Prior to entering paralle

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

On Mon, Mar 1, 2021 at 9:08 AM Greg Nancarrow wrote: > > Posting an updated set of patches that includes Amit Langote's patch > to the partition tracking scheme... > Few comments: == 1. "Prior to entering parallel-mode for execution of INSERT with parallel SELECT, a TransactionId is a

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

On Mon, Mar 1, 2021 at 12:38 PM Greg Nancarrow wrote: > On Fri, Feb 26, 2021 at 5:50 PM Amit Langote wrote: > > > > On Fri, Feb 26, 2021 at 3:35 PM Greg Nancarrow wrote: > > > On Fri, Feb 26, 2021 at 4:07 PM Amit Langote > > > wrote: > > > > The attached patch fixes this, although I am startin

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

From: Tsunakawa, Takayuki/綱川 貴之 >the current patch showd nice performance improvement in some (many?) patterns. > >So, I think it can be committed in PG 14, when it has addressed the plan cache >issue that Amit Langote-san posed. Agreed. I summarized my test results for the current patch(V18)

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

From: Hou, Zhijie/侯 志杰 > After doing some more tests on it (performance degradation will not happen > when source table is out of order). > I think we can say the performance degradation is related to the order of the > data in source table. ... > So, the order of data 's influence seems a normal

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

On Fri, Feb 26, 2021 at 3:35 PM Greg Nancarrow wrote: > On Fri, Feb 26, 2021 at 4:07 PM Amit Langote wrote: > > The attached patch fixes this, although I am starting to have second > > thoughts about how we're tracking partitions in this patch. Wondering > > if we should bite the bullet and add

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

On Fri, Feb 26, 2021 at 4:07 PM Amit Langote wrote: > > Hi Greg, > > Replying to an earlier email in the thread because I think I found a > problem relevant to the topic that was brought up. > > On Wed, Feb 17, 2021 at 10:34 PM Amit Langote wrote: > > On Wed, Feb 17, 2021 at 10:44 AM Greg Nancarr

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

> 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: Parallel INSERT (INTO ... SELECT ...)

Hi Greg, Replying to an earlier email in the thread because I think I found a problem relevant to the topic that was brought up. On Wed, Feb 17, 2021 at 10:34 PM Amit Langote wrote: > On Wed, Feb 17, 2021 at 10:44 AM Greg Nancarrow wrote: > > Is the use of "table_close(rel, NoLock)'' intentiona

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

On Wed, Feb 24, 2021 at 6:03 PM Amit Kapila wrote: > On Wed, Feb 24, 2021 at 2:14 PM Greg Nancarrow wrote: > > On Wed, Feb 24, 2021 at 3:12 PM Amit Kapila wrote: > > > On Wed, Feb 24, 2021 at 8:41 AM Greg Nancarrow > > > wrote: > > > > On Tue, Feb 23, 2021 at 10:53 PM Amit Kapila > > > > wro

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

> > 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 ...)

On Thu, Feb 25, 2021 at 12:19 AM Amit Kapila wrote: > > On Wed, Feb 24, 2021 at 6:21 PM Greg Nancarrow wrote: > > > > On Wed, Feb 24, 2021 at 10:38 PM Amit Kapila > > wrote: > > > > > > > > Thanks, I'll try it. > > > > I did, however, notice a few concerns with your suggested alternative > > >

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

On Wed, Feb 24, 2021 at 6:21 PM Greg Nancarrow wrote: > > On Wed, Feb 24, 2021 at 10:38 PM Amit Kapila wrote: > > > > > > Thanks, I'll try it. > > > I did, however, notice a few concerns with your suggested alternative fix: > > > - It is not restricted to INSERT (as current fix is). > > > > > > >

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

On Wed, Feb 24, 2021 at 10:38 PM Amit Kapila wrote: > > > > Thanks, I'll try it. > > I did, however, notice a few concerns with your suggested alternative fix: > > - It is not restricted to INSERT (as current fix is). > > > > So what? The Select also has a similar problem. > Yes, but you're poten

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

On Wed, Feb 24, 2021 at 4:30 PM Greg Nancarrow wrote: > > On Wed, Feb 24, 2021 at 8:39 PM Amit Kapila wrote: > > > > On Fri, Feb 19, 2021 at 6:56 AM Greg Nancarrow wrote: > > > > > > Posting a new version of the patches, with the following updates: > > > > > > > I am not happy with the below cod

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

On Wed, Feb 24, 2021 at 8:39 PM Amit Kapila wrote: > > On Fri, Feb 19, 2021 at 6:56 AM Greg Nancarrow wrote: > > > > Posting a new version of the patches, with the following updates: > > > > I am not happy with the below code changes, I think we need a better > way to deal with this. > > @@ -313,

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

On Fri, Feb 19, 2021 at 6:56 AM Greg Nancarrow wrote: > > Posting a new version of the patches, with the following updates: > I am not happy with the below code changes, I think we need a better way to deal with this. @@ -313,19 +314,35 @@ standard_planner(Query *parse, const char *query_string,

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

On Wed, Feb 24, 2021 at 2:14 PM Greg Nancarrow wrote: > > On Wed, Feb 24, 2021 at 3:12 PM Amit Kapila wrote: > > > > On Wed, Feb 24, 2021 at 8:41 AM Greg Nancarrow wrote: > > > > > > On Tue, Feb 23, 2021 at 10:53 PM Amit Kapila > > > wrote: > > > > > > > > > But the non-parallel plan was chose

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

On Wed, Feb 24, 2021 at 3:12 PM Amit Kapila wrote: > > On Wed, Feb 24, 2021 at 8:41 AM Greg Nancarrow wrote: > > > > On Tue, Feb 23, 2021 at 10:53 PM Amit Kapila > > wrote: > > > > > > > But the non-parallel plan was chosen (instead of a parallel plan) > > > > because of parallel-safety checks

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

On Wed, Feb 24, 2021 at 8:41 AM Greg Nancarrow wrote: > > On Tue, Feb 23, 2021 at 10:53 PM Amit Kapila wrote: > > > > > But the non-parallel plan was chosen (instead of a parallel plan) > > > because of parallel-safety checks on the partitions, which found > > > attributes of the partitions which

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

On Tue, Feb 23, 2021 at 10:53 PM Amit Kapila wrote: > > On Tue, Feb 23, 2021 at 4:47 PM Greg Nancarrow wrote: > > > > On Tue, Feb 23, 2021 at 2:30 PM Amit Kapila wrote: > > > > > > On Tue, Feb 23, 2021 at 6:37 AM Greg Nancarrow > > > wrote: > > > > > > > > On Tue, Feb 23, 2021 at 12:33 AM Amit

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

On Tue, Feb 23, 2021 at 4:47 PM Greg Nancarrow wrote: > > On Tue, Feb 23, 2021 at 2:30 PM Amit Kapila wrote: > > > > On Tue, Feb 23, 2021 at 6:37 AM Greg Nancarrow wrote: > > > > > > On Tue, Feb 23, 2021 at 12:33 AM Amit Kapila > > > wrote: > > > > > > > > On Mon, Feb 22, 2021 at 8:41 AM Greg

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

On Tue, Feb 23, 2021 at 2:30 PM Amit Kapila wrote: > > On Tue, Feb 23, 2021 at 6:37 AM Greg Nancarrow wrote: > > > > On Tue, Feb 23, 2021 at 12:33 AM Amit Kapila > > wrote: > > > > > > On Mon, Feb 22, 2021 at 8:41 AM Greg Nancarrow > > > wrote: > > > > > > > > On Fri, Feb 19, 2021 at 9:38 PM

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

On Tue, Feb 23, 2021 at 6:37 AM Greg Nancarrow wrote: > > On Tue, Feb 23, 2021 at 12:33 AM Amit Kapila wrote: > > > > On Mon, Feb 22, 2021 at 8:41 AM Greg Nancarrow wrote: > > > > > > On Fri, Feb 19, 2021 at 9:38 PM Amit Kapila > > > wrote: > > > > > > > > On Thu, Feb 18, 2021 at 11:05 AM Amit

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

On Tue, Feb 23, 2021 at 12:33 AM Amit Kapila wrote: > > On Mon, Feb 22, 2021 at 8:41 AM Greg Nancarrow wrote: > > > > On Fri, Feb 19, 2021 at 9:38 PM Amit Kapila wrote: > > > > > > On Thu, Feb 18, 2021 at 11:05 AM Amit Langote > > > wrote: > > > > > > > > > > It also occurred to me that we can

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

On Mon, Feb 22, 2021 at 8:41 AM Greg Nancarrow wrote: > > On Fri, Feb 19, 2021 at 9:38 PM Amit Kapila wrote: > > > > On Thu, Feb 18, 2021 at 11:05 AM Amit Langote > > wrote: > > > > > > > > It also occurred to me that we can avoid pointless adding of > > > > > partitions if the final plan won't

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

On Mon, Feb 22, 2021 at 8:46 AM Amit Langote wrote: > > On Fri, Feb 19, 2021 at 7:38 PM Amit Kapila wrote: > > On Thu, Feb 18, 2021 at 11:05 AM Amit Langote > > wrote: > > > > > > > > It also occurred to me that we can avoid pointless adding of > > > > > partitions if the final plan won't use p

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

On Mon, Feb 22, 2021 at 6:25 PM houzj.f...@fujitsu.com wrote: > > Hi > > (I may be wrong here) > I noticed that the patch does not have check for partial index(index > predicate). > It seems parallel index build will check it like the following: > -- > /* > * Determine if

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

> 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: Parallel INSERT (INTO ... SELECT ...)

On Fri, Feb 19, 2021 at 7:38 PM Amit Kapila wrote: > On Thu, Feb 18, 2021 at 11:05 AM Amit Langote wrote: > > > > > > It also occurred to me that we can avoid pointless adding of > > > > partitions if the final plan won't use parallelism. For that, the > > > > patch adds checking glob->parallelM

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

On Fri, Feb 19, 2021 at 9:38 PM Amit Kapila wrote: > > On Thu, Feb 18, 2021 at 11:05 AM Amit Langote wrote: > > > > > > It also occurred to me that we can avoid pointless adding of > > > > partitions if the final plan won't use parallelism. For that, the > > > > patch adds checking glob->paralle

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

From: Amit Kapila > 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 offer

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

On Fri, Feb 19, 2021 at 10:13 AM tsunakawa.ta...@fujitsu.com wrote: > > From: Greg Nancarrow > -- > On Mon, Jan 25, 2021 at 10:23 AM tsunakawa.ta...@fujitsu.com > wrote: > > (8) > > + /* > > +* If the trigger type is

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

On Thu, Feb 18, 2021 at 11:05 AM Amit Langote wrote: > > > > It also occurred to me that we can avoid pointless adding of > > > partitions if the final plan won't use parallelism. For that, the > > > patch adds checking glob->parallelModeNeeded, which seems to do the > > > trick though I don't kn

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

From: Greg Nancarrow -- On Mon, Jan 25, 2021 at 10:23 AM tsunakawa.ta...@fujitsu.com wrote: > (8) > + /* > +* If the trigger type is RI_TRIGGER_FK, this indicates a FK > exists in > +* the relation, a

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

On Thu, Feb 18, 2021 at 4:35 PM Amit Langote wrote: > > Looking at this again, I am a bit concerned about going over the whole > partition tree *twice* when making a parallel plan for insert into > partitioned tables. Maybe we should do what you did in your first > attempt a slightly differently

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

On Thu, Feb 18, 2021 at 10:03 AM Greg Nancarrow wrote: > On Thu, Feb 18, 2021 at 12:34 AM Amit Langote wrote: > > All that is to say that we should move our code to add partition OIDs > > as plan dependencies to somewhere in set_plan_references(), which > > otherwise populates PlannedStmt.relatio

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

On Thu, Feb 18, 2021 at 12:34 AM Amit Langote wrote: > > > Your revised version seems OK, though I do have a concern: > > Is the use of "table_close(rel, NoLock)'' intentional? That will keep > > the lock (lockmode) until end-of-transaction. > > I think we always keep any locks on relations that a

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

On Wed, Feb 17, 2021 at 10:44 AM Greg Nancarrow wrote: > On Wed, Feb 17, 2021 at 12:19 AM Amit Langote wrote: > > On Mon, Feb 15, 2021 at 4:39 PM Greg Nancarrow wrote: > > > On Sat, Feb 13, 2021 at 12:17 AM Amit Langote > > > wrote: > > > > On Thu, Feb 11, 2021 at 4:43 PM Greg Nancarrow > >

  1   2   3   4   >