Re: [HACKERS] Gather Merge

On Tue, Mar 14, 2017 at 9:56 AM, Tom Lane wrote: > Robert Haas writes: >> Cool, thanks for the review. I'm not quite confident that we've found >> all of the bugs here yet, but I think we're moving in the right >> direction. > > I guess the real

Re: [HACKERS] Gather Merge

Robert Haas writes: > Cool, thanks for the review. I'm not quite confident that we've found > all of the bugs here yet, but I think we're moving in the right > direction. I guess the real question here is why isn't Gather Merge more like Append and MergeAppend? That is,

Re: [HACKERS] Gather Merge

On Tue, Mar 14, 2017 at 5:47 AM, Rushabh Lathia wrote: > Thanks Robert for the patch and the explanation. > > I studied the patch and that look right to me. I performed manual testing, > run the scripts which I created during the gather merge patch also run > the tpch

Re: [HACKERS] Gather Merge

On Mon, Mar 13, 2017 at 10:56 PM, Robert Haas wrote: > On Fri, Mar 10, 2017 at 7:59 AM, Rushabh Lathia > wrote: > > Error coming from create_gather_merge_plan() from below condition: > > > > if (memcmp(sortColIdx, gm_plan->sortColIdx, > >

Re: [HACKERS] Gather Merge

On Fri, Mar 10, 2017 at 7:59 AM, Rushabh Lathia wrote: > Error coming from create_gather_merge_plan() from below condition: > > if (memcmp(sortColIdx, gm_plan->sortColIdx, >numsortkeys * sizeof(AttrNumber)) != 0) > elog(ERROR, "GatherMerge

Re: [HACKERS] Gather Merge

Error coming from create_gather_merge_plan() from below condition: if (memcmp(sortColIdx, gm_plan->sortColIdx, numsortkeys * sizeof(AttrNumber)) != 0) elog(ERROR, "GatherMerge child's targetlist doesn't match GatherMerge"); Above condition checks the sort column

Re: [HACKERS] Gather Merge

On Fri, Mar 10, 2017 at 4:09 PM, Kuntal Ghosh wrote: > On Fri, Mar 10, 2017 at 3:04 PM, Rushabh Lathia > wrote: > > > > > > On Fri, Mar 10, 2017 at 2:42 PM, Andreas Joseph Krogh < > andr...@visena.com> > > wrote: > >> > >> På fredag 10. mars

Re: [HACKERS] Gather Merge

På fredag 10. mars 2017 kl. 10:34:48, skrev Rushabh Lathia < rushabh.lat...@gmail.com >:     On Fri, Mar 10, 2017 at 2:42 PM, Andreas Joseph Krogh > wrote: På fredag 10. mars 2017 kl. 10:09:22, skrev Rushabh Lathia

Re: [HACKERS] Gather Merge

On Fri, Mar 10, 2017 at 2:42 PM, Andreas Joseph Krogh wrote: > På fredag 10. mars 2017 kl. 10:09:22, skrev Rushabh Lathia < > rushabh.lat...@gmail.com>: > > > > On Fri, Mar 10, 2017 at 2:33 PM, Andreas Joseph Krogh > wrote: >> >> [...] >> The

Re: [HACKERS] Gather Merge

På fredag 10. mars 2017 kl. 10:09:22, skrev Rushabh Lathia < rushabh.lat...@gmail.com >:     On Fri, Mar 10, 2017 at 2:33 PM, Andreas Joseph Krogh > wrote: [...] The execution-plan seems (unsurprisingly) to depend

Re: [HACKERS] Gather Merge

On Fri, Mar 10, 2017 at 2:33 PM, Andreas Joseph Krogh wrote: > På fredag 10. mars 2017 kl. 09:53:47, skrev Rushabh Lathia < > rushabh.lat...@gmail.com>: > > > > On Fri, Mar 10, 2017 at 1:44 PM, Andreas Joseph Krogh > wrote: >> >> På torsdag 09. mars 2017

Re: [HACKERS] Gather Merge

På fredag 10. mars 2017 kl. 09:53:47, skrev Rushabh Lathia < rushabh.lat...@gmail.com >:     On Fri, Mar 10, 2017 at 1:44 PM, Andreas Joseph Krogh > wrote: På torsdag 09. mars 2017 kl. 18:09:45, skrev Robert Haas

Re: [HACKERS] Gather Merge

On Fri, Mar 10, 2017 at 1:44 PM, Andreas Joseph Krogh wrote: > På torsdag 09. mars 2017 kl. 18:09:45, skrev Robert Haas < > robertmh...@gmail.com>: > > On Thu, Mar 9, 2017 at 11:25 AM, Rushabh Lathia > wrote: > > I don't see this failure with the

Re: [HACKERS] Gather Merge

På torsdag 09. mars 2017 kl. 18:09:45, skrev Robert Haas >: On Thu, Mar 9, 2017 at 11:25 AM, Rushabh Lathia wrote: > I don't see this failure with the patch. Even I forced the gather merge > in the above query and

Re: [HACKERS] Gather Merge

On Thu, Mar 9, 2017 at 11:25 AM, Rushabh Lathia wrote: > I don't see this failure with the patch. Even I forced the gather merge > in the above query and that just working fine. > > Attaching patch, with the discussed changes. Committed. -- Robert Haas EnterpriseDB:

Re: [HACKERS] Gather Merge

On Thu, Mar 9, 2017 at 9:42 PM, Robert Haas wrote: > On Thu, Mar 9, 2017 at 8:21 AM, Rushabh Lathia > wrote: > > Thanks Robert for committing this. > > > > My colleague Neha Sharma found one regression with the patch. I was about > > to send this

Re: [HACKERS] Gather Merge

On Thu, Mar 9, 2017 at 8:21 AM, Rushabh Lathia wrote: > Thanks Robert for committing this. > > My colleague Neha Sharma found one regression with the patch. I was about > to send this mail and noticed that you committed the patch. Oops. Bad timing. > postgres=#

Re: [HACKERS] Gather Merge

On Thu, Mar 9, 2017 at 6:19 PM, Robert Haas wrote: > On Wed, Mar 8, 2017 at 11:59 PM, Rushabh Lathia > wrote: > > Here is another version of patch with the suggested changes. > > Committed. > > Thanks Robert for committing this. My colleague

Re: [HACKERS] Gather Merge

On Wed, Mar 8, 2017 at 11:59 PM, Rushabh Lathia wrote: > Here is another version of patch with the suggested changes. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Gather Merge

On Thu, Mar 9, 2017 at 8:40 AM, Robert Haas wrote: > On Mon, Feb 20, 2017 at 1:35 AM, Rushabh Lathia > wrote: > > Thanks Amit for raising this point. I was not at all aware of > mark/restore. > > I tried to come up with the case, but haven't

Re: [HACKERS] Gather Merge

On Mon, Feb 20, 2017 at 1:35 AM, Rushabh Lathia wrote: > Thanks Amit for raising this point. I was not at all aware of mark/restore. > I tried to come up with the case, but haven't found such case. > > For now here is the patch with comment update. Sorry for the delay

Re: [HACKERS] Gather Merge

On Mon, Feb 20, 2017 at 1:58 PM, Dilip Kumar wrote: > On Mon, Feb 20, 2017 at 12:05 PM, Rushabh Lathia > wrote: >> Thanks Amit for raising this point. I was not at all aware of mark/restore. >> I tried to come up with the case, but haven't found

Re: [HACKERS] Gather Merge

On Mon, Feb 20, 2017 at 12:05 PM, Rushabh Lathia wrote: > Thanks Amit for raising this point. I was not at all aware of mark/restore. > I tried to come up with the case, but haven't found such case. > > For now here is the patch with comment update. I think for

Re: [HACKERS] Gather Merge

Thanks Amit for raising this point. I was not at all aware of mark/restore. I tried to come up with the case, but haven't found such case. For now here is the patch with comment update. Thanks, On Sun, Feb 19, 2017 at 7:30 PM, Amit Kapila wrote: > On Sun, Feb 19,

Re: [HACKERS] Gather Merge

On Sun, Feb 19, 2017 at 2:22 PM, Robert Haas wrote: > On Sat, Feb 18, 2017 at 6:43 PM, Amit Kapila wrote: >> I think there is a value in supporting mark/restore position for any >> node which produces sorted results, however, if you don't want to

Re: [HACKERS] Gather Merge

On Sat, Feb 18, 2017 at 6:43 PM, Amit Kapila wrote: > I think there is a value in supporting mark/restore position for any > node which produces sorted results, however, if you don't want to > support it, then I think we should update above comment in the code to > note

Re: [HACKERS] Gather Merge

On Fri, Feb 17, 2017 at 6:27 PM, Rushabh Lathia wrote: > On Fri, Feb 17, 2017 at 4:47 PM, Amit Kapila > wrote: >> >> Are you suggesting that commit 0c2070ce has helped to improve >> performance if so, I don't think that has been proved? I guess

Re: [HACKERS] Gather Merge

On Fri, Feb 17, 2017 at 4:47 PM, Amit Kapila wrote: > On Fri, Feb 17, 2017 at 3:59 PM, Thomas Munro > wrote: > > On Thu, Feb 2, 2017 at 2:32 AM, Rushabh Lathia > wrote: > >> Please find attached latest patch. > >

Re: [HACKERS] Gather Merge

On Fri, Feb 17, 2017 at 3:59 PM, Thomas Munro wrote: > On Thu, Feb 2, 2017 at 2:32 AM, Rushabh Lathia > wrote: > > Please find attached latest patch. > > The latest patch still applies (with some fuzz), builds and the > regression tests

Re: [HACKERS] Gather Merge

On Fri, Feb 17, 2017 at 3:59 PM, Thomas Munro wrote: > On Thu, Feb 2, 2017 at 2:32 AM, Rushabh Lathia > wrote: >> Please find attached latest patch. > > The latest patch still applies (with some fuzz), builds and the > regression tests

Re: [HACKERS] Gather Merge

On Thu, Feb 2, 2017 at 2:32 AM, Rushabh Lathia wrote: > Please find attached latest patch. The latest patch still applies (with some fuzz), builds and the regression tests pass. I see that Robert made a number of changes and posted a v6 along with some numbers which he

Re: [HACKERS] Gather Merge

Hi, I have done some testing with the latest patch 1)./pgbench postgres -i -F 100 -s 20 2) update pgbench_accounts set filler = 'foo' where aid%10 = 0; 3) vacuum analyze pgbench_accounts; 4) set max_parallel_workers_per_gather = 4; 5) set max_parallel_workers = 4; *Machine Configuration :-* RAM

Re: [HACKERS] Gather Merge

Due to recent below commit, patch not getting apply cleanly on master branch. commit d002f16c6ec38f76d1ee97367ba6af3000d441d0 Author: Tom Lane Date: Mon Jan 30 17:15:42 2017 -0500 Add a regression test script dedicated to exercising system views. Please find attached

Re: [HACKERS] Gather Merge

I am sorry for the delay, here is the latest re-based patch. my colleague Neha Sharma, reported one regression with the patch, where explain output for the Sort node under GatherMerge was always showing cost as zero: explain analyze select '' AS "xxx" from pgbench_accounts where filler like

Re: [HACKERS] Gather Merge

On Mon, Jan 23, 2017 at 6:51 PM, Kuntal Ghosh wrote: > On Wed, Jan 18, 2017 at 11:31 AM, Rushabh Lathia > wrote: >> > The patch needs a rebase after the commit 69f4b9c85f168ae006929eec4. Is an update going to be provided? I have moved this

Re: [HACKERS] Gather Merge

On Wed, Jan 18, 2017 at 11:31 AM, Rushabh Lathia wrote: > The patch needs a rebase after the commit 69f4b9c85f168ae006929eec4. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Gather Merge

On Tue, Jan 17, 2017 at 6:44 PM, Amit Kapila wrote: > On Tue, Jan 17, 2017 at 5:19 PM, Rushabh Lathia > wrote: > > > > > > > > Here is the benchmark number which I got with the latest (v6) patch: > > > > - max_worker_processes = DEFAULT (8) > >

Re: [HACKERS] Gather Merge

On Tue, Jan 17, 2017 at 4:26 AM, Rushabh Lathia wrote: > Another observation is, HashAggregate (case 1) is performs better compare to > GroupAggregate (case 2), but still it doesn't justify the cost difference of > those two. It may not be the only issue, or even the

Re: [HACKERS] Gather Merge

On Tue, Jan 17, 2017 at 5:19 PM, Rushabh Lathia wrote: > > > > Here is the benchmark number which I got with the latest (v6) patch: > > - max_worker_processes = DEFAULT (8) > - max_parallel_workers_per_gather = 4 > - Cold cache environment is ensured. With every query

Re: [HACKERS] Gather Merge

On Tue, Jan 17, 2017 at 5:19 PM, Rushabh Lathia wrote: > > > On Fri, Jan 13, 2017 at 10:52 AM, Rushabh Lathia > wrote: > >> >> >> On Thu, Jan 12, 2017 at 8:50 AM, Robert Haas >> wrote: >> >>> On Sun, Dec 4, 2016 at

Re: [HACKERS] Gather Merge

On Fri, Jan 13, 2017 at 10:52 AM, Rushabh Lathia wrote: > > > On Thu, Jan 12, 2017 at 8:50 AM, Robert Haas > wrote: > >> On Sun, Dec 4, 2016 at 7:36 PM, Haribabu Kommi >> wrote: >> > On Thu, Nov 24, 2016 at 11:12 PM,

Re: [HACKERS] Gather Merge

On Thu, Jan 12, 2017 at 8:50 AM, Robert Haas wrote: > On Sun, Dec 4, 2016 at 7:36 PM, Haribabu Kommi > wrote: > > On Thu, Nov 24, 2016 at 11:12 PM, Rushabh Lathia < > rushabh.lat...@gmail.com> > > wrote: > >> PFA latest patch with fix as well as

Re: [HACKERS] Gather Merge

On Sun, Dec 4, 2016 at 7:36 PM, Haribabu Kommi wrote: > On Thu, Nov 24, 2016 at 11:12 PM, Rushabh Lathia > wrote: >> PFA latest patch with fix as well as few cosmetic changes. > > Moved to next CF with "needs review" status. I spent quite a

Re: [HACKERS] Gather Merge

On Thu, Nov 24, 2016 at 11:12 PM, Rushabh Lathia wrote: > > PFA latest patch with fix as well as few cosmetic changes. > > Moved to next CF with "needs review" status. Regards, Hari Babu Fujitsu Australia

Re: [HACKERS] Gather Merge

On Wed, Nov 16, 2016 at 3:10 PM, Rushabh Lathia wrote: > > > On Mon, Nov 14, 2016 at 3:51 PM, Thomas Munro < > thomas.mu...@enterprisedb.com> wrote: > >> On Sat, Nov 12, 2016 at 1:56 AM, Rushabh Lathia >> wrote: >> > On Fri, Nov 4, 2016 at

Re: [HACKERS] Gather Merge

On Mon, Nov 14, 2016 at 3:51 PM, Thomas Munro wrote: > On Sat, Nov 12, 2016 at 1:56 AM, Rushabh Lathia > wrote: > > On Fri, Nov 4, 2016 at 8:30 AM, Thomas Munro < > thomas.mu...@enterprisedb.com> > > wrote: > >> + * Portions Copyright

Re: [HACKERS] Gather Merge

On Sat, Nov 12, 2016 at 1:56 AM, Rushabh Lathia wrote: > On Fri, Nov 4, 2016 at 8:30 AM, Thomas Munro > wrote: >> + * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group >> + * Portions Copyright (c) 1994, Regents of the

Re: [HACKERS] Gather Merge

Oops forgot to attach latest patch in the earlier mail. On Fri, Nov 11, 2016 at 6:26 PM, Rushabh Lathia wrote: > > > On Fri, Nov 4, 2016 at 8:30 AM, Thomas Munro < > thomas.mu...@enterprisedb.com> wrote: > >> On Thu, Oct 27, 2016 at 10:50 PM, Rushabh Lathia >>

Re: [HACKERS] Gather Merge

On Fri, Nov 4, 2016 at 8:30 AM, Thomas Munro wrote: > On Thu, Oct 27, 2016 at 10:50 PM, Rushabh Lathia > wrote: > > Please find attached latest patch which fix the review point as well as > > additional clean-up. > > I've signed up to

Re: [HACKERS] Gather Merge

On Fri, Nov 4, 2016 at 8:30 AM, Thomas Munro wrote: > On Thu, Oct 27, 2016 at 10:50 PM, Rushabh Lathia > wrote: >> Please find attached latest patch which fix the review point as well as >> additional clean-up. > > +/* > + * Read the tuple

Re: [HACKERS] Gather Merge

On Sat, Nov 5, 2016 at 2:42 AM, Tom Lane wrote: > Michael Paquier writes: >> On Fri, Nov 4, 2016 at 12:00 PM, Thomas Munro >> wrote: >>> Shouldn't this say just "(c) 2016, PostgreSQL Global Development >>> Group"?

Re: [HACKERS] Gather Merge

On Sat, Nov 5, 2016 at 1:55 AM, Robert Haas wrote: > On Thu, Nov 3, 2016 at 11:00 PM, Thomas Munro > wrote: >> + /* >> + * Avoid log(0)... >> + */ >> + N = (path->num_workers < 2) ? 2.0 : (double) path->num_workers; >> + logN = LOG2(N); >>

Re: [HACKERS] Gather Merge

Michael Paquier writes: > On Fri, Nov 4, 2016 at 12:00 PM, Thomas Munro > wrote: >> Shouldn't this say just "(c) 2016, PostgreSQL Global Development >> Group"? Are we supposed to be blaming the University of California >> for new files?

Re: [HACKERS] Gather Merge

On Thu, Nov 3, 2016 at 11:00 PM, Thomas Munro wrote: > + /* > + * Avoid log(0)... > + */ > + N = (path->num_workers < 2) ? 2.0 : (double) path->num_workers; > + logN = LOG2(N); > ... > + /* Per-tuple heap maintenance cost */ > + run_cost += path->path.rows *

Re: [HACKERS] Gather Merge

On Fri, Nov 4, 2016 at 12:00 PM, Thomas Munro wrote: > + * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group > + * Portions Copyright (c) 1994, Regents of the University of California > > Shouldn't this say just "(c) 2016, PostgreSQL Global

Re: [HACKERS] Gather Merge

On Thu, Oct 27, 2016 at 10:50 PM, Rushabh Lathia wrote: > Please find attached latest patch which fix the review point as well as > additional clean-up. I've signed up to review this patch and I'm planning to do some testing. Here's some initial feedback after a quick

Re: [HACKERS] Gather Merge

Please find attached latest patch which fix the review point as well as additional clean-up. - Get rid of funnel_slot as its not needed for the Gather Merge - renamed fill_tuple_array to form_tuple_array - Fix possible infinite loop into gather_merge_init (Reported by Amit Kaplia) - Fix tqueue.c

Re: [HACKERS] Gather Merge

On Thu, Oct 20, 2016 at 1:12 PM, Amit Kapila wrote: > On Tue, Oct 18, 2016 at 5:29 PM, Rushabh Lathia > wrote: > > On Mon, Oct 17, 2016 at 2:26 PM, Amit Kapila > > wrote: > >> > >> There is lot of common code between

Re: [HACKERS] Gather Merge

On Thu, Oct 20, 2016 at 12:22 AM, Peter Geoghegan wrote: > On Tue, Oct 4, 2016 at 11:05 PM, Rushabh Lathia > wrote: > > Query 4: With GM 7901.480 -> Without GM 9064.776 > > Query 5: With GM 53452.126 -> Without GM 55059.511 > > Query 9: With GM

Re: [HACKERS] Gather Merge

On Tue, Oct 18, 2016 at 5:29 PM, Rushabh Lathia wrote: > On Mon, Oct 17, 2016 at 2:26 PM, Amit Kapila > wrote: >> >> There is lot of common code between ExecGatherMerge and ExecGather. >> Do you think it makes sense to have a common function to

Re: [HACKERS] Gather Merge

On Tue, Oct 4, 2016 at 11:05 PM, Rushabh Lathia wrote: > Query 4: With GM 7901.480 -> Without GM 9064.776 > Query 5: With GM 53452.126 -> Without GM 55059.511 > Query 9: With GM 52613.132 -> Without GM 98206.793 > Query 15: With GM 68051.058 -> Without GM 68918.378 >

Re: [HACKERS] Gather Merge

Thanks Amit for reviewing this patch. On Mon, Oct 17, 2016 at 2:26 PM, Amit Kapila wrote: > On Wed, Oct 5, 2016 at 11:35 AM, Rushabh Lathia > wrote: > > Hi hackers, > > > > Attached is the patch to implement Gather Merge. > > > > Couple of

Re: [HACKERS] Gather Merge

On Mon, Oct 17, 2016 at 4:56 AM, Amit Kapila wrote: > + node->nreaders < 2) ... > I see there are small discrepancies in both the codes like I don't see > the use of single_copy flag, as it is present in gather node. single_copy doesn't make sense for GatherMerge,

Re: [HACKERS] Gather Merge

On Wed, Oct 5, 2016 at 11:35 AM, Rushabh Lathia wrote: > Hi hackers, > > Attached is the patch to implement Gather Merge. > Couple of review comments: 1. ExecGatherMerge() { .. + /* No workers? Then never mind. */ + if (!got_any_worker || + node->nreaders < 2) + { +