With no solution on the horizon, and the author saying it fixes some of his trigger queries, I am inclined to apply this. I don't see any downside except for some extra pfrees if we ever fix this.
--------------------------------------------------------------------------- Dmitry Tkach wrote: > 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 > -- Bruce Momjian | http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 ---------------------------(end of broadcast)--------------------------- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
