Oliver Jowett <oliver@opencloud.com> writes:
>> Perhaps PerformCursorOpen should copy the query tree before planning, or
>> plan in a different memory context?
> Patch attached. It moves query planning inside the new portal's memory
> context. With this applied I can run Barry's testcase without errors,
> and valgrind seems OK with it too.
I think the better solution is the first way (copy the querytree first).
The problem with the way you did it is that all the temporary structures
built by the planner will be left behind in the cursor's memory context,
and can't be reclaimed until the cursor is destroyed. In the case of a
complex query that could represent a pretty serious memory leak. It
seems better to eat the cost of copying the querytree an extra time,
especially since this way forms a patch that's easy to reverse whenever
we fix the planner to be less cavalier about scribbling on its input.
I've applied the attached patch instead (and analogously in 7.4 branch).
Would you confirm it fixes the problem you see?
regards, tom lane
*** src/backend/commands/portalcmds.c.orig Thu Sep 16 12:58:28 2004
--- src/backend/commands/portalcmds.c Sun Nov 28 17:02:22 2004
***************
*** 62,73 **** RequireTransactionChain((void *) stmt, "DECLARE CURSOR"); /* * The query has been
throughparse analysis, but not rewriting or * planning as yet. Note that the grammar ensured we have a SELECT
* query, so we are not expecting rule rewriting to do anything * strange. */
! rewritten = QueryRewrite((Query *) stmt->query); if (list_length(rewritten) != 1 || !IsA(linitial(rewritten),
Query)) elog(ERROR, "unexpected rewrite result"); query = (Query *) linitial(rewritten);
--- 62,82 ---- RequireTransactionChain((void *) stmt, "DECLARE CURSOR"); /*
+ * Because the planner is not cool about not scribbling on its input,
+ * we make a preliminary copy of the source querytree. This prevents
+ * problems in the case that the DECLARE CURSOR is in a portal and is
+ * executed repeatedly. XXX the planner really shouldn't modify its
+ * input ... FIXME someday.
+ */
+ query = copyObject(stmt->query);
+
+ /* * The query has been through parse analysis, but not rewriting or * planning as yet. Note that the
grammarensured we have a SELECT * query, so we are not expecting rule rewriting to do anything * strange.
*/
! rewritten = QueryRewrite(query); if (list_length(rewritten) != 1 || !IsA(linitial(rewritten), Query))
elog(ERROR,"unexpected rewrite result"); query = (Query *) linitial(rewritten);
