Tom Lane wrote: >Dmitry Tkach <[EMAIL PROTECTED]> writes: > >>*** nodeIndexscan.c.orig Fri Apr 19 10:29:57 2002 >>--- nodeIndexscan.c Fri Apr 19 10:30:00 2002 >>*************** >>*** 505,510 **** >>--- 505,514 ---- >> */ >> ExecClearTuple(scanstate->cstate.cs_ResultTupleSlot); >> ExecClearTuple(scanstate->css_ScanTupleSlot); >>+ pfree(scanstate); >>+ pfree(indexstate->iss_RelationDescs); >>+ pfree(indexstate->iss_ScanDescs); >>+ pfree(indexstate); >> } >> > >I do not believe this patch will fix anything. > Well... It does fix the query I mentioned in my first message (and a few others I was doing when I ran into this problem)
> >In general, the EndNode routines do not bother with releasing memory, > Well... Not quite true. For example, ExecEndIndexScan () does release lots of stuff, that was allocated in ExecInitIndexScan (), it just forgets about these four things... > >because it's the end of the query and we're about to drop or reset >the per-query context anyway. > ... if the query manages to complete without running out of memory like in my case :-) > If the above pfrees are actually needed >then there are a heck of a lot of other places that need explicit >pfrees. > Perhaps... I haven't run into any other places just yet... >And that is not a path to a solution, because there will >*always* be places that forgot a pfree. > Sure :-) You can say this about any bug pretty much :-) There will *always* be bugs... That doesn't mean that the ones, that are found should not be fixed though > What's needed is a structural >solution. > I understand what you are saying... I was looking around for one for a while, but it seems, but there doesn't seem to be anything 'more structural' for this particular case, as far as I can see - per query context does get released properly in the end, it's just that the query requires too much temporary memory to complete, so the end is actually never reached... After all, I repeat, ExecEndIndexScan () DOES free up lots of memory, so I don't see how adding these four things that got forgotten would hurt. > >I think your real complaint is that SQL functions leak memory. They >should be creating a sub-context for their queries that can be freed >when the function finishes. > I guess, you are right here - I tried using a subquery instead of a function, and that is not leaking, because it does ExecRescan () for each outer tuple, instead of reinitializing the executor completely as it is the case with sql func . fmgr_sql () DOES use it's own context, but it looks like it doesn't want to reset it every time, because of caching... Perhaps, it could just execute every command in a function as a SubPlan, instead of reinitializing every time, but that wouldn't be a simple 4 line fix, that certainly doesn't break anything that was working before :-) I was thinking in terms of a quick PATCH, that can fix a particular problem, and would be safe to be put in. It does not prevent any "structural solution" from being implemented if somebody comes up with one in the future, and it would STILL be useful even when then solution is implemented, because it makes memory usage more conservative, thus reducing the load on system resources... Frankly, I don't see what is your problem with it at all :-) Dima ---------------------------(end of broadcast)--------------------------- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
