Thread: don't allocate HashAgg hash tables when running explain only
Hi, I got somewhat scared when my explain took a few seconds to complete and used a few gigs of RAM. To reproduce try the following: discard temp; create temp table a as select to_timestamp(generate_series(1, 7000)) i; analyze a; set work_mem to '3GB'; explain select distinct a1.i - a2.i from a a1, a a2; I would appreciate if someone could have a look at the patch attached, which makes executor skip initializing hash tables when doing explain only. Best, Alex
Attachment
On 13/11/2020 18:10, Alexey Bashtanov wrote: > Hi, > > I got somewhat scared when my explain took a few seconds to complete and > used a few gigs of RAM. > To reproduce try the following: > > discard temp; > create temp table a as select to_timestamp(generate_series(1, 7000)) i; > analyze a; > set work_mem to '3GB'; > explain select distinct a1.i - a2.i from a a1, a a2; > > I would appreciate if someone could have a look at the patch attached, > which makes executor skip initializing hash tables when doing explain only. Makes sense. Committed, thanks for the patch! - Heikki
On Wed, 18 Nov 2020 at 05:40, Heikki Linnakangas <hlinnaka@iki.fi> wrote: > > On 13/11/2020 18:10, Alexey Bashtanov wrote: >> > I would appreciate if someone could have a look at the patch attached, > > which makes executor skip initializing hash tables when doing explain only. > > Makes sense. Committed, thanks for the patch! Egads. That seems like a backpatchable bug fix to me. Have we been doing this all along?! -- greg
On 19/11/2020 07:20, Greg Stark wrote: > On Wed, 18 Nov 2020 at 05:40, Heikki Linnakangas <hlinnaka@iki.fi> wrote: >> >> On 13/11/2020 18:10, Alexey Bashtanov wrote: >>>> I would appreciate if someone could have a look at the patch attached, >>> which makes executor skip initializing hash tables when doing explain only. >> >> Makes sense. Committed, thanks for the patch! > > Egads. That seems like a backpatchable bug fix to me. Have we been > doing this all along?! Yeah, I believe it's always been like that. Yeah, arguably it should be backpatched. I felt conservative and didn't backpatch, but feel free to do it if you think it should be. - Heikki
On Thu, Nov 19, 2020 at 08:47:51AM +0200, Heikki Linnakangas wrote: > Yeah, I believe it's always been like that. Yeah, arguably it should be > backpatched. I felt conservative and didn't backpatch, but feel free to do > it if you think it should be. +1 for a backpatch. The difference in runtime for EXPLAIN in this case is quite something. -- Michael
Attachment
On 20/11/2020 08:31, Michael Paquier wrote: > On Thu, Nov 19, 2020 at 08:47:51AM +0200, Heikki Linnakangas wrote: >> Yeah, I believe it's always been like that. Yeah, arguably it should be >> backpatched. I felt conservative and didn't backpatch, but feel free to do >> it if you think it should be. > > +1 for a backpatch. The difference in runtime for EXPLAIN in this case > is quite something. That's two votes for backpatching. Ok, I'll go do it. - Heikki