From: Tom Lane <tgl@sss.pgh.pa.us>
To: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
cc: pgsql-hackers@postgresql.org
Subject: Re: list_free in addRangeTableEntryForJoin
In-reply-to: <ff7f8032-4f39-4df0-9941-b4b7e03f67c3@tantorlabs.com>
References: <ff7f8032-4f39-4df0-9941-b4b7e03f67c3@tantorlabs.com>
Comments: In-reply-to Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
	message dated "Mon, 10 Jun 2024 13:35:39 +0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <735132.1718034979.1@sss.pgh.pa.us>
Date: Mon, 10 Jun 2024 11:56:19 -0400
Message-ID: <735133.1718034979@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/735133.1718034979%40sss.pgh.pa.us>
Precedence: bulk

Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> writes:
> I have identified a potential memory leak in the 
> `addRangeTableEntryForJoin()` function. The second parameter of 
> `addRangeTableEntryForJoin()`, `colnames`, is a `List*` that is 
> concatenated with another `List*`, `eref->colnames`, using 
> `list_concat()`. We need to pass only the last `numaliases` elements of 
> the list, for which we use `list_copy_tail`. This function creates a 
> copy of the `colnames` list, resulting in `colnames` pointing to the 
> current list that will not be freed. Consequently, a new list is already 
> concatenated.

> To address this issue, I have invoked `list_free(colnames)` afterwards. 

Sadly, I think this is basically a waste of development effort.
The parser, like the planner and rewriter and many other Postgres
subsystems, leaks tons of small allocations like this.  That's
*by design*, and *it's okay*, because we run these steps in short-
lived memory contexts that will be reclaimed in toto as soon as
the useful output data structures are no longer needed.  It's not
worth the sort of intellectual effort you've put in in this thread
to prove whether individual small structures are no longer needed.
Plus, in many cases that isn't obvious, and/or it'd be notationally
messy to reclaim things explicitly at a suitable point.

If we were talking about a potentially-very-large data structure,
or one that we might create very many instances of during one
parsing pass, then it might be worth the trouble to free explicitly;
but I don't see that concern applying here.

You might find src/backend/utils/mmgr/README to be worth reading.

			regards, tom lane