Message-ID: <44F0AA23.9030800@joeconway.com>
Date: Sat, 26 Aug 2006 13:08:03 -0700
From: Joe Conway <mail@joeconway.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
	rv:1.7.13) Gecko/20060501 Fedora/1.7.13-1.1.fc5
MIME-Version: 1.0
To: Bruce Momjian <bruce@momjian.us>
CC: Tom Lane <tgl@sss.pgh.pa.us>,
	"Hackers (PostgreSQL)" <pgsql-hackers@postgresql.org>
Subject: Re: VALUES clause memory optimization
References: <200608260335.k7Q3ZeJ04955@momjian.us>
In-Reply-To: <200608260335.k7Q3ZeJ04955@momjian.us>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Bruce Momjian wrote:
> Has this been addressed?
> 
>>Tom Lane wrote:
>>>The reason we could safely list_free inside transformInsertRow is that
>>>we know all its callers have just built the passed-in list and so there
>>>are no other pointers to it.  That doesn't apply in the general case of
>>>grammar output.

After another look, even in this case there could be other pointers. 
Starting around line 667 of analyze.c:

8<------------------
     foreach(lc, selectQuery->targetList)
     {
         TargetEntry *tle = (TargetEntry *) lfirst(lc);
         Expr	   *expr;

         if (tle->resjunk)
             continue;
         if (tle->expr &&
             (IsA(tle->expr, Const) ||IsA(tle->expr, Param)) &&
             exprType((Node *) tle->expr) == UNKNOWNOID)
             expr = tle->expr;
         else
             expr = (Expr *) makeVar(rtr->rtindex,
                                     tle->resno,
                                     exprType((Node *) tle->expr),
                                     exprTypmod((Node *) tle->expr),
                                     0);
          exprList = lappend(exprList, expr);
     }

     /* Prepare row for assignment to target table */
     exprList = transformInsertRow(pstate, exprList,
                                   stmt->cols,
                                   icolumns, attrnos);

8<------------------

So in the UNKNOWNOID case, it wouldn't be safe to free the node after 
transformation because it originates in the grammar output. The *only* 
safe thing to free up would be a the input exprList itself. Not much to 
be saved there.

>>>My advice is to get that low-hanging fruit
>>>in transformInsertRow and leave the other ideas for 8.3.

This one case doesn't provide that much memory savings by itself anyway. 
I guess we'll just live with it until we can come up with a safe way to 
free grammar output.

Joe