Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-07 Thread Tomas Vondra
On 02/06/2016 10:22 PM, Tom Lane wrote: Tomas Vondra writes: What about using the dense allocation even for the skew buckets, but not one context for all skew buckets but one per bucket? Then when we delete a bucket, we simply destroy the context (and free the

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-07 Thread Robert Haas
On Sat, Feb 6, 2016 at 12:47 PM, Tom Lane wrote: > So I'm of the opinion that a great deal more work is needed here. > But it's not something we're going to be able to get done for 9.5.1, > or realistically 9.5.anything. Whereas adding additional klugery to >

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-07 Thread Tom Lane
Robert Haas writes: > On Sat, Feb 6, 2016 at 12:47 PM, Tom Lane wrote: >> So I'm of the opinion that a great deal more work is needed here. >> But it's not something we're going to be able to get done for 9.5.1, >> or realistically 9.5.anything.

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tom Lane
Tomas Vondra writes: > I believe the attached patch should fix this by actually copying the > tuples into the densely allocated chunks. Haven't tested it though, will > do in a few hours. BTW, I confirmed that this patch fixes the wrong-number-of-output-tuples

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tomas Vondra
On 02/06/2016 08:39 PM, Andres Freund wrote: On 2016-02-06 20:34:07 +0100, Tomas Vondra wrote: On 02/06/2016 06:47 PM, Tom Lane wrote: * It incorporates a bespoke reimplementation of palloc into hash joins. This is not a maintainable/scalable way to go about reducing memory consumption. It

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tom Lane
Tomas Vondra writes: > On 02/06/2016 08:39 PM, Andres Freund wrote: >> FWIW, I've done that at some point. Noticeable speedups (that's what >> I cared about), but a bit annoying to use. There's many random >> pfree()s around, and then there's MemoryContextContains(),

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tom Lane
Tomas Vondra writes: > On 02/06/2016 06:47 PM, Tom Lane wrote: >> I note also that while the idea of ExecHashRemoveNextSkewBucket is >> to reduce memory consumed by the skew table to make it available to >> the main hash table, in point of fact it's unlikely that the

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tomas Vondra
On 02/06/2016 09:55 PM, Tom Lane wrote: Tomas Vondra writes: On 02/06/2016 06:47 PM, Tom Lane wrote: I note also that while the idea of ExecHashRemoveNextSkewBucket is to reduce memory consumed by the skew table to make it available to the main hash table, in

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tom Lane
Tomas Vondra writes: > What about using the dense allocation even for the skew buckets, but not > one context for all skew buckets but one per bucket? Then when we delete > a bucket, we simply destroy the context (and free the chunks, just like > we do with the

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tom Lane
Tomas Vondra writes: > On 02/06/2016 02:34 AM, Tom Lane wrote: >> I have sussed what's happening in bug #13908. Basically, commit >> 45f6240a8fa9d355 ("Pack tuples in a hash join batch densely, to save >> memory") broke things for the case where a hash join is using

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tomas Vondra
On 02/06/2016 06:47 PM, Tom Lane wrote: Tomas Vondra writes: On 02/06/2016 02:34 AM, Tom Lane wrote: I have sussed what's happening in bug #13908. Basically, commit 45f6240a8fa9d355 ("Pack tuples in a hash join batch densely, to save memory") broke things for the

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Andres Freund
On 2016-02-06 20:34:07 +0100, Tomas Vondra wrote: > On 02/06/2016 06:47 PM, Tom Lane wrote: > >* It incorporates a bespoke reimplementation of palloc into hash > >joins. This is not a maintainable/scalable way to go about reducing > >memory consumption. It should have been done with an

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-06 Thread Tomas Vondra
On 02/06/2016 02:34 AM, Tom Lane wrote: I have sussed what's happening in bug #13908. Basically, commit 45f6240a8fa9d355 ("Pack tuples in a hash join batch densely, to save memory") broke things for the case where a hash join is using a skew table. The reason is that that commit only changed the

Re: [HACKERS] Explanation for bug #13908: hash joins are badly broken

2016-02-05 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: > I'm of the opinion that this is a stop-ship bug for 9.5.1. Barring > somebody producing a fix over the weekend, I will deal with it by > reverting the aforementioned commit. Agreed. Thanks! Stephen signature.asc Description: Digital signature