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
Attachments:
dont-build-agg-hashtable-on-explain-only-v1.difftext/x-patch; charset=UTF-8; name=dont-build-agg-hashtable-on-explain-only-v1.diffDownload
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index d87677d659..196fe09c52 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3665,7 +3665,11 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
&aggstate->hash_ngroups_limit,
&aggstate->hash_planned_partitions);
find_hash_columns(aggstate);
- build_hash_tables(aggstate);
+
+ /* Skip massive memory allocation when running only explain */
+ if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
+ build_hash_tables(aggstate);
+
aggstate->table_filled = false;
/* Initialize this to 1, meaning nothing spilled, yet */
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.
--
MichaelOn 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