From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15321: XMLTABLE leaks memory like crazy
In-Reply-To: <153394403528.10284.7530399040974170549@wrigleys.postgresql.org>
	(PG Bug reporting form's message of "Fri, 10 Aug 2018 23:33:55 +0000")
Message-ID: <87va8h7nyl.fsf@news-spur.riddles.org.uk>
References: <153394403528.10284.7530399040974170549@wrigleys.postgresql.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Sat, 11 Aug 2018 01:28:47 +0100
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

>>>>> "PG" == PG Bug reporting form <noreply@postgresql.org> writes:

 PG> From a report on IRC:

 PG> XMLTABLE runs a lot of setup code in the per-query memory context -
 PG> resulting in allocations of copies of namespace names, other
 PG> values, and _multiple copies of the passed-in XML document_, which
 PG> are not freed anywhere.

Alvaro, I think this comment of yours from when you committed this work
is relevant:

 >> I just pushed XMLTABLE, after some additional changes. Please test
 >> it thoroughly and report any problems.
 [...]
 >> Some changes I made:
 >> * I removed the "buildercxt" memory context. It seemed mostly
 >> pointless, and I was disturbed by the MemoryContextResetOnly().
 >> Per-value memory still uses the per-value memory context, but the
 >> rest of the stuff is in the per-query context, which should be
 >> pretty much the same.

A quick reading suggests that the per-value context should have been
changed to not be a child of "buildercxt" (which would avoid the
MemoryContextResetOnly issue - that's a good sign that you've put a
child context under the wrong parent). But the use of the per-query
context instead is exactly what causes this blowup; compare with what
nodeFunctionscan does with its "argcontext" (and the corresponding
comments in ExecMakeTableFunctionResult).

-- 
Andrew (irc:RhodiumToad)