> During anonymous block execution in the function "plpgsql_estate_setup", a > local casting hash table gets created in SPI memory context. When hash > table look up is performed in "get_cast_hashenty" function if entry is no > present , memory is allocated in CacheMemoryContext in function > "GetCachedExpression".At the end of proc execution SPI memory context is > deleted and hence local hash table gets deleted, but still entries remain > in Cachemeorycontext.
Yeah, it's from using just a short-lived cast hash table for DO blocks. I think that was okay when it was written, but when we wheeled the CachedExpression machinery undeneath it, we created a problem.
> Please find attached(memoryleakfix.patch) to this email.
I don't think this fix is acceptable at all. A minor problem is that we can't change the API of GetCachedExpression() in stable branches, because extensions may be using it. We could work around that by making it a wrapper function. But the big problem is that this patch destroys the reason for using a CachedExpression in the first place: because you aren't linking it into cached_expression_list, the plancache will not detect events that should obsolete the expression. The test cases added by 04fe805a1 only cover regular functions, but one that did domain constraint DDL within a DO block would expose the shortcoming.
A possible answer is to split plpgsql's cast hash table into two parts. The lower-level part would contain the hash key, cast_expr and cast_cexpr fields, and would have session lifespan and be used by both DO blocks and regular functions. In this way we'd not leak CachedExpressions. The upper-level hash table would contain the hash key, a link to the relevant lower-level entry, and the cast_exprstate, cast_in_use, cast_lxid fields. There would be a session-lifespan one of these plus one for each DO block, so that management of the ExprStates still works as it does now for DO blocks.
This could be factored in other ways, and maybe another way would be simpler. But the idea is that DO blocks should use persistent CachedExpressions even though their cast_exprstates are transient.
