Re: [HACKERS] parallelize queries containing initplans

2017-11-11 Thread Amit Kapila
On Sat, Nov 11, 2017 at 12:15 AM, Robert Haas wrote: > On Tue, Nov 7, 2017 at 4:45 AM, Amit Kapila wrote: >> As mentioned, changed the status of the patch in CF app. > > I spent some time reviewing this patch today and found myself still > quite uncomfortable with the fact that it was adding exec

Re: [HACKERS] parallelize queries containing initplans

2017-11-10 Thread Tom Lane
Robert Haas writes: > On Fri, Nov 10, 2017 at 2:24 PM, Tom Lane wrote: >> Also, I wonder whether the InvalidOid hack in SS_assign_special_param >> requires commentary. It might be safer to use a valid type OID there, >> perhaps VOIDOID or INTERNALOID. > There is existing precedent for using Inv

Re: [HACKERS] parallelize queries containing initplans

2017-11-10 Thread Robert Haas
On Fri, Nov 10, 2017 at 2:24 PM, Tom Lane wrote: > Robert Haas writes: >> I decided to try instead teaching the planner to keep track of the >> types of PARAM_EXEC parameters as they were created, and that seems to >> work fine. See 0001, attached. > > I did not look at the other part, but 0001

Re: [HACKERS] parallelize queries containing initplans

2017-11-10 Thread Tom Lane
Robert Haas writes: > I decided to try instead teaching the planner to keep track of the > types of PARAM_EXEC parameters as they were created, and that seems to > work fine. See 0001, attached. I did not look at the other part, but 0001 looks reasonable to me. I might quibble with the grammar i

Re: [HACKERS] parallelize queries containing initplans

2017-11-10 Thread Robert Haas
On Tue, Nov 7, 2017 at 4:45 AM, Amit Kapila wrote: > As mentioned, changed the status of the patch in CF app. I spent some time reviewing this patch today and found myself still quite uncomfortable with the fact that it was adding execution-time work to track the types of parameters - types that

Re: [HACKERS] parallelize queries containing initplans

2017-11-07 Thread Amit Kapila
On Mon, Oct 30, 2017 at 9:00 AM, Amit Kapila wrote: > On Wed, Oct 11, 2017 at 9:24 PM, Robert Haas wrote: >> On Mon, Oct 9, 2017 at 5:56 AM, Amit Kapila wrote: >>> How about always returning false for PARAM_EXTERN? >> >> Yeah, I think that's what we should do. Let's do that first as a >> separa

Re: [HACKERS] parallelize queries containing initplans

2017-11-03 Thread Amit Kapila
On Mon, Oct 30, 2017 at 10:07 AM, Robert Haas wrote: > On Mon, Oct 30, 2017 at 9:00 AM, Amit Kapila wrote: >> Now that the PARAM_EXTERN issue is fixed, I have rebased this patch. >> This patch had been switched to Ready For Committer in last CF, then >> Robert had comments which I have addressed,

Re: [HACKERS] parallelize queries containing initplans

2017-10-31 Thread tushar
On 10/30/2017 01:36 PM, tushar wrote: On 10/30/2017 09:02 AM, Amit Kapila wrote: Thanks a lot Tushar for testing this patch.  In the latest patch, I have just rebased some comments, there is no code change, so I don't expect any change in behavior, but feel free to test it once again. Thanks A

Re: [HACKERS] parallelize queries containing initplans

2017-10-30 Thread tushar
On 10/30/2017 09:02 AM, Amit Kapila wrote: Thanks a lot Tushar for testing this patch. In the latest patch, I have just rebased some comments, there is no code change, so I don't expect any change in behavior, but feel free to test it once again. Thanks Amit. Sure. -- regards,tushar Enterpris

Re: [HACKERS] parallelize queries containing initplans

2017-10-29 Thread Robert Haas
On Mon, Oct 30, 2017 at 9:00 AM, Amit Kapila wrote: > Now that the PARAM_EXTERN issue is fixed, I have rebased this patch. > This patch had been switched to Ready For Committer in last CF, then > Robert had comments which I have addressed, so I think the status > should be switched back to Ready F

Re: [HACKERS] parallelize queries containing initplans

2017-10-29 Thread Amit Kapila
On Wed, Oct 18, 2017 at 2:06 PM, tushar wrote: > On 10/11/2017 12:42 PM, tushar wrote: >> >> On 10/09/2017 03:26 PM, Amit Kapila wrote: >>> >>> I have reverted the check >>> in the attached patch. >> >> >> I have applied this patch against PG HEAD and run sqlsmith and analyzed >> results . didn't

Re: [HACKERS] parallelize queries containing initplans

2017-10-29 Thread Amit Kapila
On Wed, Oct 11, 2017 at 9:24 PM, Robert Haas wrote: > On Mon, Oct 9, 2017 at 5:56 AM, Amit Kapila wrote: >> How about always returning false for PARAM_EXTERN? > > Yeah, I think that's what we should do. Let's do that first as a > separate patch, which we might even want to back-patch, then retur

Re: [HACKERS] parallelize queries containing initplans

2017-10-12 Thread Amit Kapila
On Wed, Oct 11, 2017 at 9:24 PM, Robert Haas wrote: > On Mon, Oct 9, 2017 at 5:56 AM, Amit Kapila wrote: >> How about always returning false for PARAM_EXTERN? > > Yeah, I think that's what we should do. Let's do that first as a > separate patch, which we might even want to back-patch, then retur

Re: [HACKERS] parallelize queries containing initplans

2017-10-11 Thread Robert Haas
On Mon, Oct 9, 2017 at 5:56 AM, Amit Kapila wrote: > How about always returning false for PARAM_EXTERN? Yeah, I think that's what we should do. Let's do that first as a separate patch, which we might even want to back-patch, then return to this. -- Robert Haas EnterpriseDB: http://www.enterpri

Re: [HACKERS] parallelize queries containing initplans

2017-10-11 Thread tushar
On 10/09/2017 03:26 PM, Amit Kapila wrote: I have reverted the check in the attached patch. I have applied this patch against PG HEAD and run sqlsmith and analyzed results . didn't find any specific failures against this patch. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ Th

Re: [HACKERS] parallelize queries containing initplans

2017-10-09 Thread Amit Kapila
On Sat, Oct 7, 2017 at 7:22 PM, Robert Haas wrote: > On Fri, Oct 6, 2017 at 7:08 AM, Amit Kapila wrote: >> I have fixed the other review comment related to using safe_param_list >> in the attached patch. I think I have fixed all comments given by >> you, but let me know if I have missed anything

Re: [HACKERS] parallelize queries containing initplans

2017-10-07 Thread Robert Haas
On Fri, Oct 6, 2017 at 7:08 AM, Amit Kapila wrote: > I have fixed the other review comment related to using safe_param_list > in the attached patch. I think I have fixed all comments given by > you, but let me know if I have missed anything or you have any other > comment. -Param *

Re: [HACKERS] parallelize queries containing initplans

2017-10-06 Thread Amit Kapila
On Fri, Oct 6, 2017 at 2:32 AM, Robert Haas wrote: > On Thu, Oct 5, 2017 at 1:16 PM, Amit Kapila wrote: >> On Thu, Oct 5, 2017 at 6:08 PM, Robert Haas wrote: >>> On Thu, Oct 5, 2017 at 5:52 AM, Amit Kapila wrote: Now, unless, I am missing something here, it won't be possible to detect

Re: [HACKERS] parallelize queries containing initplans

2017-10-05 Thread Robert Haas
On Thu, Oct 5, 2017 at 1:16 PM, Amit Kapila wrote: > On Thu, Oct 5, 2017 at 6:08 PM, Robert Haas wrote: >> On Thu, Oct 5, 2017 at 5:52 AM, Amit Kapila wrote: >>> Now, unless, I am missing something here, it won't be possible to >>> detect params in such cases during forming of join rels and henc

Re: [HACKERS] parallelize queries containing initplans

2017-10-05 Thread Amit Kapila
On Thu, Oct 5, 2017 at 6:08 PM, Robert Haas wrote: > On Thu, Oct 5, 2017 at 5:52 AM, Amit Kapila wrote: >> Now, unless, I am missing something here, it won't be possible to >> detect params in such cases during forming of join rels and hence we >> need the tests in generate_gather_paths. Let me

Re: [HACKERS] parallelize queries containing initplans

2017-10-05 Thread Robert Haas
On Thu, Oct 5, 2017 at 5:52 AM, Amit Kapila wrote: > Now, unless, I am missing something here, it won't be possible to > detect params in such cases during forming of join rels and hence we > need the tests in generate_gather_paths. Let me know if I am missing > something in this context or if yo

Re: [HACKERS] parallelize queries containing initplans

2017-10-05 Thread Amit Kapila
On Wed, Oct 4, 2017 at 12:55 PM, Amit Kapila wrote: > On Wed, Oct 4, 2017 at 3:40 AM, Robert Haas wrote: >> On Tue, Oct 3, 2017 at 7:33 AM, Amit Kapila wrote: >> >> Having said all that, I think that this patch only wants to handle the >> subset of cases (2) and (4) where the relevant InitPlan i

Re: [HACKERS] parallelize queries containing initplans

2017-10-04 Thread Amit Kapila
On Wed, Oct 4, 2017 at 12:55 PM, Amit Kapila wrote: > On Wed, Oct 4, 2017 at 3:40 AM, Robert Haas wrote: >> On Tue, Oct 3, 2017 at 7:33 AM, Amit Kapila wrote: >> >> Having said all that, I think that this patch only wants to handle the >> subset of cases (2) and (4) where the relevant InitPlan i

Re: [HACKERS] parallelize queries containing initplans

2017-10-04 Thread Amit Kapila
On Wed, Oct 4, 2017 at 3:40 AM, Robert Haas wrote: > On Tue, Oct 3, 2017 at 7:33 AM, Amit Kapila wrote: > > Having said all that, I think that this patch only wants to handle the > subset of cases (2) and (4) where the relevant InitPlan is attached > ABOVE the Gather node -- which seems very reas

Re: [HACKERS] parallelize queries containing initplans

2017-10-03 Thread Robert Haas
On Tue, Oct 3, 2017 at 7:33 AM, Amit Kapila wrote: >> - Why do we even need contains_parallel_unsafe_param() and >> is_initplan_is_below_current_query_level() in the first place, anyway? >> I think there's been some discussion of that on this thread, but I'm >> not sure I completely understand it

Re: [HACKERS] parallelize queries containing initplans

2017-10-03 Thread Amit Kapila
On Mon, Oct 2, 2017 at 8:13 PM, Robert Haas wrote: > On Thu, Sep 14, 2017 at 10:45 PM, Amit Kapila wrote: >> The latest patch again needs to be rebased. Find rebased patch >> attached with this email. > > I read through this patch this morning. Copying Tom in the hopes > that he might chime in

Re: [HACKERS] parallelize queries containing initplans

2017-10-02 Thread Robert Haas
On Thu, Sep 14, 2017 at 10:45 PM, Amit Kapila wrote: > The latest patch again needs to be rebased. Find rebased patch > attached with this email. I read through this patch this morning. Copying Tom in the hopes that he might chime in on the following two issues in particular: 1. Is there any

Re: [HACKERS] parallelize queries containing initplans

2017-10-01 Thread Daniel Gustafsson
> On 15 Sep 2017, at 04:45, Amit Kapila wrote: > > On Thu, Aug 31, 2017 at 11:23 AM, Amit Kapila wrote: >> On Mon, Aug 21, 2017 at 2:40 PM, Amit Kapila wrote: >>> On Mon, Aug 21, 2017 at 1:44 PM, Haribabu Kommi >>> wrote: Thanks for adding more details. It is easy to understan

Re: [HACKERS] parallelize queries containing initplans

2017-09-14 Thread Amit Kapila
On Thu, Aug 31, 2017 at 11:23 AM, Amit Kapila wrote: > On Mon, Aug 21, 2017 at 2:40 PM, Amit Kapila wrote: >> On Mon, Aug 21, 2017 at 1:44 PM, Haribabu Kommi >> wrote: >>> >>> >>> Thanks for adding more details. It is easy to understand. >>> >>> I marked the patch as ready for committer in the c

Re: [HACKERS] parallelize queries containing initplans

2017-08-30 Thread Amit Kapila
On Mon, Aug 21, 2017 at 2:40 PM, Amit Kapila wrote: > On Mon, Aug 21, 2017 at 1:44 PM, Haribabu Kommi > wrote: >> >> >> Thanks for adding more details. It is easy to understand. >> >> I marked the patch as ready for committer in the commitfest. >> Rebased the patch. The output of test case adde

Re: [HACKERS] parallelize queries containing initplans

2017-08-21 Thread Amit Kapila
On Mon, Aug 21, 2017 at 1:44 PM, Haribabu Kommi wrote: > > > Thanks for adding more details. It is easy to understand. > > I marked the patch as ready for committer in the commitfest. > Thank you. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hacke

Re: [HACKERS] parallelize queries containing initplans

2017-08-21 Thread Haribabu Kommi
On Mon, Aug 14, 2017 at 8:41 PM, Amit Kapila wrote: > On Sun, Aug 13, 2017 at 6:49 PM, Haribabu Kommi > wrote: > > On Fri, Aug 11, 2017 at 1:18 AM, Amit Kapila > > wrote: > >> > > > > Thanks for the updated patch. Patch looks fine. I just have some > > minor comments. > > > > + * ExecEvalParamE

Re: [HACKERS] parallelize queries containing initplans

2017-08-14 Thread Amit Kapila
On Sun, Aug 13, 2017 at 6:49 PM, Haribabu Kommi wrote: > On Fri, Aug 11, 2017 at 1:18 AM, Amit Kapila > wrote: >> > > Thanks for the updated patch. Patch looks fine. I just have some > minor comments. > > + * ExecEvalParamExecParams > + * > + * Execute the subplan stored in PARAM_EXEC initplans p

Re: [HACKERS] parallelize queries containing initplans

2017-08-13 Thread Haribabu Kommi
On Fri, Aug 11, 2017 at 1:18 AM, Amit Kapila wrote: > On Wed, Aug 9, 2017 at 6:51 PM, Haribabu Kommi > wrote: > > > > > > + if (IsA(plan, Gather)) > > + ((Gather *) plan)->initParam = bms_intersect(plan->lefttree->extParam, > > initSetParam); > > + else if (IsA(plan, GatherMerge)) > > + ((Gather

Re: [HACKERS] parallelize queries containing initplans

2017-08-10 Thread Amit Kapila
On Wed, Aug 9, 2017 at 6:51 PM, Haribabu Kommi wrote: > > On Wed, Aug 9, 2017 at 9:26 PM, Amit Kapila wrote: >> > > By the way, I tested the patch with by DML support for parallel patch to > check the returning of clause of insert, and all the returning clause init > plans > are parallel plans wi

Re: [HACKERS] parallelize queries containing initplans

2017-08-09 Thread Haribabu Kommi
On Wed, Aug 9, 2017 at 9:26 PM, Amit Kapila wrote: > On Wed, Aug 9, 2017 at 10:24 AM, Haribabu Kommi > wrote: > > > > > > For the following query the parallel plan is not chosen. The query > contains > > an init plan that refer the outer node. > > > > We don't want to generate the parallel plan

Re: [HACKERS] parallelize queries containing initplans

2017-08-09 Thread Haribabu Kommi
On Wed, Aug 9, 2017 at 8:54 PM, Kuntal Ghosh wrote: > On Wed, Aug 9, 2017 at 10:24 AM, Haribabu Kommi > wrote: > > > > I tested the latest patch and the parallel plan is getting choose for > most > > of > > the init plans. > > > Thanks for testing. > > > For the following query the parallel plan

Re: [HACKERS] parallelize queries containing initplans

2017-08-09 Thread Amit Kapila
On Wed, Aug 9, 2017 at 10:24 AM, Haribabu Kommi wrote: > > > On Mon, Jul 17, 2017 at 10:53 PM, Amit Kapila > wrote: >> >> On Tue, Mar 28, 2017 at 7:25 AM, Amit Kapila >> wrote: >> > On Thu, Mar 16, 2017 at 2:34 AM, Kuntal Ghosh >> > wrote: >> >> On Tue, Mar 14, 2017 at 3:20 PM, Amit Kapila >>

Re: [HACKERS] parallelize queries containing initplans

2017-08-09 Thread Kuntal Ghosh
On Wed, Aug 9, 2017 at 10:24 AM, Haribabu Kommi wrote: > > I tested the latest patch and the parallel plan is getting choose for most > of > the init plans. > Thanks for testing. > For the following query the parallel plan is not chosen. The query contains > an init plan that refer the outer node

Re: [HACKERS] parallelize queries containing initplans

2017-08-08 Thread Haribabu Kommi
On Mon, Jul 17, 2017 at 10:53 PM, Amit Kapila wrote: > On Tue, Mar 28, 2017 at 7:25 AM, Amit Kapila > wrote: > > On Thu, Mar 16, 2017 at 2:34 AM, Kuntal Ghosh > > wrote: > >> On Tue, Mar 14, 2017 at 3:20 PM, Amit Kapila > wrote: > >>> Based on that idea, I have modified the patch such that it

Re: [HACKERS] parallelize queries containing initplans

2017-07-17 Thread Amit Kapila
On Tue, Mar 28, 2017 at 7:25 AM, Amit Kapila wrote: > On Thu, Mar 16, 2017 at 2:34 AM, Kuntal Ghosh > wrote: >> On Tue, Mar 14, 2017 at 3:20 PM, Amit Kapila wrote: >>> Based on that idea, I have modified the patch such that it will >>> compute the set of initplans Params that are required below

Re: [HACKERS] parallelize queries containing initplans

2017-03-27 Thread Amit Kapila
On Thu, Mar 16, 2017 at 2:34 AM, Kuntal Ghosh wrote: > On Tue, Mar 14, 2017 at 3:20 PM, Amit Kapila wrote: >> Based on that idea, I have modified the patch such that it will >> compute the set of initplans Params that are required below gather >> node and store them as bitmap of initplan params a

Re: [HACKERS] parallelize queries containing initplans

2017-03-15 Thread Kuntal Ghosh
On Tue, Mar 14, 2017 at 3:20 PM, Amit Kapila wrote: > Based on that idea, I have modified the patch such that it will > compute the set of initplans Params that are required below gather > node and store them as bitmap of initplan params at gather node. > During set_plan_references, we can find th

Re: [HACKERS] parallelize queries containing initplans

2017-03-14 Thread Amit Kapila
On Fri, Feb 10, 2017 at 4:34 PM, Amit Kapila wrote: > > I could see two possibilities to determine whether the plan (for which > we are going to generate an initplan) contains a reference to a > correlated var param node. One is to write a plan or path walker to > determine any such reference and

Re: [HACKERS] parallelize queries containing initplans

2017-02-10 Thread Amit Kapila
On Tue, Jan 31, 2017 at 4:16 PM, Amit Kapila wrote: > On Wed, Dec 28, 2016 at 5:20 PM, Amit Kapila wrote: > >> The drawback of the second approach is >> that we need to evaluate the initplan before it is actually required >> which means that we might evaluate it even when it is not required. I

Re: [HACKERS] parallelize queries containing initplans

2017-01-31 Thread Amit Kapila
On Wed, Dec 28, 2016 at 5:20 PM, Amit Kapila wrote: > To start > with let us see the plan of TPC-H query (Q-22) and understand how it > can be improved. > > Limit >InitPlan 1 (returns $0) > -> Finalize Aggregate >-> Gather > Workers Planned: 2 >