From 58c47ccfff20b8c125903482725c1dbfd30beade Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 29 Feb 2020 13:48:09 -0500 Subject: Correctly re-use hash tables in buildSubPlanHash(). Commit 356687bd8 omitted to remove leftover code for destroying a hashed subplan's hash tables, with the result that the tables were always rebuilt not reused; this leads to severe memory leakage if a hashed subplan is re-executed enough times. Moreover, the code for reusing the hashnulls table had a typo that would have made it do the wrong thing if it were reached. Looking at the code coverage report shows severe under-coverage of the potential callers of ResetTupleHashTable, so add some test cases that exercise them. Andreas Karlsson and Tom Lane, per reports from Ranier Vilela and Justin Pryzby. Backpatch to v11, as the faulty commit was. Discussion: https://postgr.es/m/edb62547-c453-c35b-3ed6-a069e4d6b937@proxel.se Discussion: https://postgr.es/m/CAEudQAo=DCebm1RXtig9OH+QivpS97sMkikt0A9qHmMUs+g6ZA@mail.gmail.com Discussion: https://postgr.es/m/20200210032547.GA1412@telsasoft.com --- src/backend/executor/nodeSubplan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/backend/executor/nodeSubplan.c') diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index ff953178797..298b7757f57 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -495,8 +495,6 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) * need to store subplan output rows that contain NULL. */ MemoryContextReset(node->hashtablecxt); - node->hashtable = NULL; - node->hashnulls = NULL; node->havehashrows = false; node->havenullrows = false; @@ -533,7 +531,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) } if (node->hashnulls) - ResetTupleHashTable(node->hashtable); + ResetTupleHashTable(node->hashnulls); else node->hashnulls = BuildTupleHashTableExt(node->parent, node->descRight, @@ -549,6 +547,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) node->hashtempcxt, false); } + else + node->hashnulls = NULL; /* * We are probably in a short-lived expression-evaluation context. Switch -- cgit v1.2.3