Re: [JDBC] Strange server error with current 8.0beta driver - Mailing list pgsql-hackers

Tom,

Your patch works for my test cases.  Thanks to both you and Oliver for
getting this fixed.

--Barry

-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Sunday, November 28, 2004 2:23 PM
To: Oliver Jowett
Cc: Barry Lind; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] [JDBC] Strange server error with current 8.0beta
driver

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 through parse 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 grammar ensured 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);