I was rather bemused to discover just now that the node-freeing
functions in nodes/freefuncs.c are never called from anywhere;
in fact, the module hasn't got a single exported entry point!

(I expect that freeObject() is supposed to be an external entry
point; perhaps it got demoted to a static during one of Bruce's
periodic get-rid-of-unreferenced-global-symbols passes.)

So much for all that tedious labor to maintain the freeXXX functions
every time we update a node type ;-)

Now I am not quite sure what to do.  I was intending to use freeObject
to clean up rule qual/action trees during relcache flush --- up to now,
that cache data has been permanently leaked by any relcache flush
affecting a relation with rules.  But if freefuncs.c hasn't actually
been used in a long time, it may well be suffering serious bit-rot.
I am worried about turning it on just before beta.  I am especially
worried about turning it on for use only in a seldom-taken code path ---
if there are bugs in it, we may not find them until after release.

Should I chicken out and let the memory leak persist until we start
7.1 development cycle?  Or go for it and hope the code's OK?

			regards, tom lane

Sat, 29 Jan 2000 17:51:27 -0500
Tom Lane <tgl@sss.pgh.pa.us>
freefuncs.c is never called from anywhere!?
> I was rather bemused to discover just now that the node-freeing
> functions in nodes/freefuncs.c are never called from anywhere;
> in fact, the module hasn't got a single exported entry point!
> 
> (I expect that freeObject() is supposed to be an external entry
> point; perhaps it got demoted to a static during one of Bruce's
> periodic get-rid-of-unreferenced-global-symbols passes.)
> 
> So much for all that tedious labor to maintain the freeXXX functions
> every time we update a node type ;-)

Are you sure about this?  I thought these things were called from
macros.

However, I can't find any macro that uses this like makeNode does with
its pasteing.  I would perhaps move it to a _deadcode directory and see
what happens.  Or should we enable it?


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sat, 29 Jan 2000 22:18:09 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> I was rather bemused to discover just now that the node-freeing
>> functions in nodes/freefuncs.c are never called from anywhere;
>> in fact, the module hasn't got a single exported entry point!

> Are you sure about this?  I thought these things were called from
> macros.

nm --defined --extern shows that freefuncs.o exports no symbols.
Therefore it's impossible for any outside code to refer to it.
(Hmm, I wonder if any other modules are equally dead?)

> I would perhaps move it to a _deadcode directory and see
> what happens.  Or should we enable it?

I certainly don't want to bit-bucket it --- I was just very surprised
that it's not currently being used.

After further consideration I realized that if RelationClearRelation
uses freeObject() to flush rules, then a DROP on a view will exercise
the code.  So it's not quite as hard to test as I was thinking before.
I'm leaning towards the "go for it" answer at the moment...

			regards, tom lane

Sat, 29 Jan 2000 22:49:19 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
> -----Original Message-----
> From: owner-pgsql-hackers@postgreSQL.org
> [mailto:owner-pgsql-hackers@postgreSQL.org]On Behalf Of Tom Lane
> 
> I was rather bemused to discover just now that the node-freeing
> functions in nodes/freefuncs.c are never called from anywhere;
> in fact, the module hasn't got a single exported entry point!
> 
> (I expect that freeObject() is supposed to be an external entry
> point; perhaps it got demoted to a static during one of Bruce's
> periodic get-rid-of-unreferenced-global-symbols passes.)
> 
> So much for all that tedious labor to maintain the freeXXX functions
> every time we update a node type ;-)
>

As far as I see,freeObject() has a fundamental problem.
Probably it couldn't free multiple references safely.

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp

Sun, 30 Jan 2000 15:05:13 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
RE: [HACKERS] freefuncs.c is never called from anywhere!?
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> As far as I see,freeObject() has a fundamental problem.
> Probably it couldn't free multiple references safely.

Yes, that code will have a problem with node trees that have
circular references or multiple links to the same node.
Both of which are cases that we can currently generate :-(.

Of course, circular links will also break copyfuncs, equalfuncs,
and printfuncs.  We have a known bug with the backend crashing
when -d is enabled for certain queries, because of infinite
recursion in printfuncs.

For the moment, the only thing I need the freefuncs code for is
to free nodetrees that have been created by stringToNode.  AFAICT
that routine is incapable of creating circular links or multiple
links, so it should work.

Eventually it would be nice to have a better solution.

			regards, tom lane

Sun, 30 Jan 2000 01:32:51 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> 
> "Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> > As far as I see,freeObject() has a fundamental problem.
> > Probably it couldn't free multiple references safely.
> 
> Yes, that code will have a problem with node trees that have
> circular references or multiple links to the same node.
> Both of which are cases that we can currently generate :-(.
> 
> Of course, circular links will also break copyfuncs, equalfuncs,
> and printfuncs.  We have a known bug with the backend crashing
> when -d is enabled for certain queries, because of infinite
> recursion in printfuncs.
>

Multiple links to the same node is not so fatal for other xxxxObject().
In fact they are used without big problem.
But isn't it fatal for freeObject() ?

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp

Mon, 31 Jan 2000 09:04:47 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
RE: [HACKERS] freefuncs.c is never called from anywhere!? 
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> Multiple links to the same node is not so fatal for other xxxxObject().
> In fact they are used without big problem.
> But isn't it fatal for freeObject() ?

Yes, of course.  We could possibly fix that by maintaining a list of
already-freed nodes during any one freeObject() call, but that seems
painful and slow.

However, the case I'm interested in would only be trying to free
nodetrees that were created by either copyObject or stringToNode,
and since neither of those routines produce multiple or circular
links, it seems safe enough to use freeObject as-is for the purpose.

			regards, tom lane

Sun, 30 Jan 2000 20:55:14 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
On Sat, 29 Jan 2000, Bruce Momjian wrote:

> However, I can't find any macro that uses this like makeNode does with
> its pasteing.  I would perhaps move it to a _deadcode directory and see
> what happens.  Or should we enable it?

Please no more _deadcode. Why do we have CVS?

(In the same spirit it would also be nice to tag NOT_USED sections with a
version number, so it could be yanked two or three releases past.)

-- 
Peter Eisentraut                  Sernanders vaeg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden


Mon, 31 Jan 2000 13:30:16 +0100 (MET)
Peter Eisentraut <e99re41@DoCS.UU.SE>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
On Mon, 31 Jan 2000, Peter Eisentraut wrote:

> On Sat, 29 Jan 2000, Bruce Momjian wrote:
> 
> > However, I can't find any macro that uses this like makeNode does with
> > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > what happens.  Or should we enable it?
> 
> Please no more _deadcode. Why do we have CVS?
> 
> (In the same spirit it would also be nice to tag NOT_USED sections with a
> version number, so it could be yanked two or three releases past.)

Why not just yank it period?  'cvs diff' will show what was yanked, and
the log message could say just 'yanked NOT_USED code from source tree'...


Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 


Mon, 31 Jan 2000 09:36:26 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
On Mon, 31 Jan 2000, The Hermit Hacker wrote:

> On Mon, 31 Jan 2000, Peter Eisentraut wrote:
> 
> > On Sat, 29 Jan 2000, Bruce Momjian wrote:
> > 
> > > However, I can't find any macro that uses this like makeNode does with
> > > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > > what happens.  Or should we enable it?
> > 
> > Please no more _deadcode. Why do we have CVS?
> > 
> > (In the same spirit it would also be nice to tag NOT_USED sections with a
> > version number, so it could be yanked two or three releases past.)
> 
> Why not just yank it period?  'cvs diff' will show what was yanked, and
> the log message could say just 'yanked NOT_USED code from source tree'...

I *suspect* the purpose of some of these sections is that if it turns out
somebody needed them for their application, we could just tell them to
re-enable them there and there. If this is the case, then it has gone way
past abuse already, though. Some parts of been NOT_USED for many years and
most likely don't work anymore.


-- 
Peter Eisentraut                  Sernanders vaeg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden


Mon, 31 Jan 2000 14:41:53 +0100 (MET)
Peter Eisentraut <e99re41@DoCS.UU.SE>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
On Mon, 31 Jan 2000, Peter Eisentraut wrote:

> On Mon, 31 Jan 2000, The Hermit Hacker wrote:
> 
> > On Mon, 31 Jan 2000, Peter Eisentraut wrote:
> > 
> > > On Sat, 29 Jan 2000, Bruce Momjian wrote:
> > > 
> > > > However, I can't find any macro that uses this like makeNode does with
> > > > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > > > what happens.  Or should we enable it?
> > > 
> > > Please no more _deadcode. Why do we have CVS?
> > > 
> > > (In the same spirit it would also be nice to tag NOT_USED sections with a
> > > version number, so it could be yanked two or three releases past.)
> > 
> > Why not just yank it period?  'cvs diff' will show what was yanked, and
> > the log message could say just 'yanked NOT_USED code from source tree'...
> 
> I *suspect* the purpose of some of these sections is that if it turns out
> somebody needed them for their application, we could just tell them to
> re-enable them there and there. If this is the case, then it has gone way
> past abuse already, though. Some parts of been NOT_USED for many years and
> most likely don't work anymore.

IMHO, if you want to take the time to do it, yank it.  we have a cvs
history of everything we do, so it isn't as if its permanently lost, and,
as you say, with all the changes that have gone around some of the
NOT_USED code, they are most likely not even valid/working anymore :)

Makes for a good time to do clean up ... before beta and/or major release
...

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 


Mon, 31 Jan 2000 10:00:51 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
Peter Eisentraut <e99re41@DoCS.UU.SE> writes:
>>>> Please no more _deadcode. Why do we have CVS?

> I *suspect* the purpose of some of these sections is that if it turns out
> somebody needed them for their application, we could just tell them to
> re-enable them there and there.

I think there are several different motivations, and it would be bad to
tar all the occurrences of NOT_USED with the same brush.

Some of them are indeed just code that was #ifdef'd out while testing
a replacement solution, and never physically removed.  I think that's what
CVS is for; we should keep history in CVS versions, not in the current
sources.

Some of them are useful code that just happens not to be referenced
at the moment, but might be needed again at any time.  (And who'd
think to go look in back CVS versions, when wondering "is there a
routine that ..."?)

Some of them are code that was disabled because it was broken, but with
hopes of fixing and re-enabling it someday.  (The expensive-function
code in the optimizer seems to be an example of this.)  The advantage
of leaving such code in current sources is that it has a better chance
of tracking global edits, such as routine or struct field renamings.

I know that I have usually ignored files in _deadcode directories when
making widespread changes, which means that those files drift further
and further away from any hope of future usefulness.  This doesn't
happen quite as fast for code that's #ifdef NOT_USED but still present
in current source files.

I think it would be a bad idea to go around and yank out NOT_USED code
wholesale --- this should be looked at on a case-by-case basis, with
consideration of whether the code might be useful again in future.

But I think I might vote for pruning the _deadcode directories
wholesale.  I suspect the files in them are suffering from
irrecoverable bit rot.

			regards, tom lane

Mon, 31 Jan 2000 10:42:04 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
> On Sat, 29 Jan 2000, Bruce Momjian wrote:
> 
> > However, I can't find any macro that uses this like makeNode does with
> > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > what happens.  Or should we enable it?
> 
> Please no more _deadcode. Why do we have CVS?

No one looks in the old cvs versions.  Better to have a separate
directory.

> 
> (In the same spirit it would also be nice to tag NOT_USED sections with a
> version number, so it could be yanked two or three releases past.)

Good idea, but most NOT_USED must be checked to see if there is anything
valuable in there.  If not, it can be removed.  Age is not the issue. 
We have code from Berkeley in NOT_USED because sometimes we may need it.
You can remove it when you understand why it was there, or what it is
doing, and know we will not need it.

We may be able to remove them all someday if some suggests that.  They
are usually quite old.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 31 Jan 2000 20:48:07 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
> On Mon, 31 Jan 2000, Peter Eisentraut wrote:
> 
> > On Sat, 29 Jan 2000, Bruce Momjian wrote:
> > 
> > > However, I can't find any macro that uses this like makeNode does with
> > > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > > what happens.  Or should we enable it?
> > 
> > Please no more _deadcode. Why do we have CVS?
> > 
> > (In the same spirit it would also be nice to tag NOT_USED sections with a
> > version number, so it could be yanked two or three releases past.)
> 
> Why not just yank it period?  'cvs diff' will show what was yanked, and
> the log message could say just 'yanked NOT_USED code from source tree'...

We will never think to look and find it again if we do that.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 31 Jan 2000 20:49:45 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
> I think it would be a bad idea to go around and yank out NOT_USED code
> wholesale --- this should be looked at on a case-by-case basis, with
> consideration of whether the code might be useful again in future.
> 
> But I think I might vote for pruning the _deadcode directories
> wholesale.  I suspect the files in them are suffering from
> irrecoverable bit rot.

I disagree on _deadcode.  While the code is rotted, it does implement
some full functions that have no other history, like verion.c for
versioning and xfunc for expensive functions.  Yanking them means we can
never know what they did.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 31 Jan 2000 20:58:27 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
On Mon, 31 Jan 2000, Bruce Momjian wrote:

> > I think it would be a bad idea to go around and yank out NOT_USED code
> > wholesale --- this should be looked at on a case-by-case basis, with
> > consideration of whether the code might be useful again in future.
> > 
> > But I think I might vote for pruning the _deadcode directories
> > wholesale.  I suspect the files in them are suffering from
> > irrecoverable bit rot.
> 
> I disagree on _deadcode.  While the code is rotted, it does implement
> some full functions that have no other history, like verion.c for
> versioning and xfunc for expensive functions.  Yanking them means we can
> never know what they did.

even 'yanked' code is still in the cvs repository ... is it something that
ppl actually refer to, or are just afraid of possibly needing/wanting to
go back and look at it?  maybe just add a '.readme' file to a subdirectory
to note files that were removed or something like that?

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 


Mon, 31 Jan 2000 22:03:41 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
On Mon, 31 Jan 2000, Bruce Momjian wrote:

> > On Sat, 29 Jan 2000, Bruce Momjian wrote:
> > 
> > > However, I can't find any macro that uses this like makeNode does with
> > > its pasteing.  I would perhaps move it to a _deadcode directory and see
> > > what happens.  Or should we enable it?
> > 
> > Please no more _deadcode. Why do we have CVS?
> 
> No one looks in the old cvs versions.  Better to have a separate
> directory.

Does anyone check the _deadcode directory? :)  I never even remember its
there until you guys bring it up to move things into it *shrug*

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 


Mon, 31 Jan 2000 22:32:19 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
> > No one looks in the old cvs versions.  Better to have a separate
> > directory.
> 
> Does anyone check the _deadcode directory? :)  I never even remember its
> there until you guys bring it up to move things into it *shrug*

We have removed some features like expensive functions, versioning and
recipe that were moved there, and could be used if we need them again.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 31 Jan 2000 21:34:43 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
The Hermit Hacker <scrappy@hub.org> writes:
>> I disagree on _deadcode.  While the code is rotted, it does implement
>> some full functions that have no other history, like verion.c for
>> versioning and xfunc for expensive functions.  Yanking them means we can
>> never know what they did.

> even 'yanked' code is still in the cvs repository ...

Precisely.  Seems to me we ought to think about our code maintenance
methods with knowledge that back versions will be available from CVS.
Keeping stuff in the current tree has some advantages if it's stuff
you think you might want again in the near term, but I think it's
the wrong way to deal with stuff that we're only keeping for historical
purposes.  For example, if I wanted to try to understand the xfunc
code, I'd really have to go back to the last version where it worked;
the partially-patched files sitting in _deadcode would most likely be
more confusing than helpful...

			regards, tom lane

Mon, 31 Jan 2000 22:08:33 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
On Mon, 31 Jan 2000, Tom Lane wrote:

> The Hermit Hacker <scrappy@hub.org> writes:
> >> I disagree on _deadcode.  While the code is rotted, it does implement
> >> some full functions that have no other history, like verion.c for
> >> versioning and xfunc for expensive functions.  Yanking them means we can
> >> never know what they did.
> 
> > even 'yanked' code is still in the cvs repository ...
> 
> Precisely.  Seems to me we ought to think about our code maintenance
> methods with knowledge that back versions will be available from CVS.
> Keeping stuff in the current tree has some advantages if it's stuff
> you think you might want again in the near term, but I think it's
> the wrong way to deal with stuff that we're only keeping for historical
> purposes.  For example, if I wanted to try to understand the xfunc
> code, I'd really have to go back to the last version where it worked;
> the partially-patched files sitting in _deadcode would most likely be
> more confusing than helpful...

have to agree here ... how much of the NOT_USED code is totally irrelevant
based on the changes around it?

Some sort of 'text log' of what is removed and date should be kept, if ppl
want to be able to go back ... basically, instead of 'moved to _deadcode',
just add a line to a text file stating 'function X removed on date Y' so
that ppl have a guide to look at the cvs repository with ...

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org 


Mon, 31 Jan 2000 23:17:37 -0400 (AST)
The Hermit Hacker <scrappy@hub.org>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
> The Hermit Hacker <scrappy@hub.org> writes:
> >> I disagree on _deadcode.  While the code is rotted, it does implement
> >> some full functions that have no other history, like verion.c for
> >> versioning and xfunc for expensive functions.  Yanking them means we can
> >> never know what they did.
> 
> > even 'yanked' code is still in the cvs repository ...
> 
> Precisely.  Seems to me we ought to think about our code maintenance
> methods with knowledge that back versions will be available from CVS.
> Keeping stuff in the current tree has some advantages if it's stuff
> you think you might want again in the near term, but I think it's
> the wrong way to deal with stuff that we're only keeping for historical
> purposes.  For example, if I wanted to try to understand the xfunc
> code, I'd really have to go back to the last version where it worked;
> the partially-patched files sitting in _deadcode would most likely be
> more confusing than helpful...

Personally, I am willing to yank it all now.  I think we understand the
code base well enough that someone who knows the code can go through and
quickly identify the 90% of NOT_USED that is just junk and remove it. 
Tom Lane, you are the only person I know who can do this.  Sorry.


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 31 Jan 2000 22:27:17 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Personally, I am willing to yank it all now.  I think we understand the
> code base well enough that someone who knows the code can go through and
> quickly identify the 90% of NOT_USED that is just junk and remove it. 
> Tom Lane, you are the only person I know who can do this.  Sorry.

Moi?  There are parts I understand well, and other parts not.
In any case I don't have time now to make such a sweep.

What I'd suggest is that we all just modify our operating procedures
a little.  Any time you have studied a bit of code long enough to be
pretty sure you understand how it works, feel free to cut out any
NOT_USED segments that you can't see any prospect of wanting again.

I've already been applying this philosophy to some extent in modules
that I've worked on, but I will try to do it on a regular basis from
here on.  If we all do it, that should eliminate the useless stuff
over time.

Next question is what to do about the _deadcode subdirectories.
For the most part I don't even know what's in 'em ... do you?

			regards, tom lane

Mon, 31 Jan 2000 22:52:37 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] freefuncs.c is never called from anywhere!? 
Any status on this?

> I was rather bemused to discover just now that the node-freeing
> functions in nodes/freefuncs.c are never called from anywhere;
> in fact, the module hasn't got a single exported entry point!
> 
> (I expect that freeObject() is supposed to be an external entry
> point; perhaps it got demoted to a static during one of Bruce's
> periodic get-rid-of-unreferenced-global-symbols passes.)
> 
> So much for all that tedious labor to maintain the freeXXX functions
> every time we update a node type ;-)
> 
> Now I am not quite sure what to do.  I was intending to use freeObject
> to clean up rule qual/action trees during relcache flush --- up to now,
> that cache data has been permanently leaked by any relcache flush
> affecting a relation with rules.  But if freefuncs.c hasn't actually
> been used in a long time, it may well be suffering serious bit-rot.
> I am worried about turning it on just before beta.  I am especially
> worried about turning it on for use only in a seldom-taken code path ---
> if there are bugs in it, we may not find them until after release.
> 
> Should I chicken out and let the memory leak persist until we start
> 7.1 development cycle?  Or go for it and hope the code's OK?
> 
> 			regards, tom lane
> 
> ************
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 06:32:06 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: freefuncs.c is never called from anywhere!?
> On Mon, 31 Jan 2000, Tom Lane wrote:
> 
> > The Hermit Hacker <scrappy@hub.org> writes:
> > >> I disagree on _deadcode.  While the code is rotted, it does implement
> > >> some full functions that have no other history, like verion.c for
> > >> versioning and xfunc for expensive functions.  Yanking them means we can
> > >> never know what they did.
> > 
> > > even 'yanked' code is still in the cvs repository ...
> > 
> > Precisely.  Seems to me we ought to think about our code maintenance
> > methods with knowledge that back versions will be available from CVS.
> > Keeping stuff in the current tree has some advantages if it's stuff
> > you think you might want again in the near term, but I think it's
> > the wrong way to deal with stuff that we're only keeping for historical
> > purposes.  For example, if I wanted to try to understand the xfunc
> > code, I'd really have to go back to the last version where it worked;
> > the partially-patched files sitting in _deadcode would most likely be
> > more confusing than helpful...
> 
> have to agree here ... how much of the NOT_USED code is totally irrelevant
> based on the changes around it?
> 
> Some sort of 'text log' of what is removed and date should be kept, if ppl
> want to be able to go back ... basically, instead of 'moved to _deadcode',
> just add a line to a text file stating 'function X removed on date Y' so
> that ppl have a guide to look at the cvs repository with ...

I am thinking of going through the code and removing NOT_USED functions
I know to be useless.  OK?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 07:21:48 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: freefuncs.c is never called from anywhere!?]
> Any status on this?

Nothing done about it yet.

IIRC, some people were concerned about the fact that freeObject()
couldn't possibly cope with circular structures, multiply-linked
subexpressions, etc.  I don't think that's a problem for my intended
use in the relcache --- the only structures I'll be freeing are ones
previously read in by the node-reading functions, and those aren't
going to have any surprises like that.

			regards, tom lane

Fri, 09 Jun 2000 10:31:50 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: freefuncs.c is never called from anywhere!? 
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I am thinking of going through the code and removing NOT_USED functions
> I know to be useless.  OK?

Don't see why not, though I'd recommend treading lightly.  That unused-
functions script tends to pull out stuff that has been exported in the
expectation that someone would want it someday, but for one reason or
another it's not being called right at the moment.  We don't really want
to delete that sort of code.

BTW, I've also been rather dubious about the automatic
demote-to-static-function script for the same reason --- when someone
comes along and needs function X, it's hard to tell whether X was
intended to be private or was intended to be public but got demoted by
the script.  It's tough to maintain clear module APIs with tools like
that second-guessing the author's intentions.

			regards, tom lane

Fri, 09 Jun 2000 10:47:56 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: freefuncs.c is never called from anywhere!?] 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I am thinking of going through the code and removing NOT_USED functions
> > I know to be useless.  OK?
> 
> Don't see why not, though I'd recommend treading lightly.  That unused-
> functions script tends to pull out stuff that has been exported in the
> expectation that someone would want it someday, but for one reason or
> another it's not being called right at the moment.  We don't really want
> to delete that sort of code.
> 
> BTW, I've also been rather dubious about the automatic
> demote-to-static-function script for the same reason --- when someone
> comes along and needs function X, it's hard to tell whether X was
> intended to be private or was intended to be public but got demoted by
> the script.  It's tough to maintain clear module APIs with tools like
> that second-guessing the author's intentions.

Well, I put a comment on one's that have to be exported, and others come
up as referenced in the regression tests.

The only one I know about is:

	GetAttributeByName
	GetAttributeByNum

and some debug functions.  The above two are the only ones that I marked
as static at one time that I later had to export.

The other advantage of static is that if a static function is not called
in the file, the compiler throws a warning and I can then mark it as
NOT_USED.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 12:52:33 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: freefuncs.c is never called from anywhere!?]
> BTW, I've also been rather dubious about the automatic
> demote-to-static-function script for the same reason --- when someone
> comes along and needs function X, it's hard to tell whether X was
> intended to be private or was intended to be public but got demoted by
> the script.  It's tough to maintain clear module APIs with tools like
> that second-guessing the author's intentions.

Also, I don't touch the interfaces, only the backend code.  There aren't
many api stuff there, except spi, which I also don't touch.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 13:10:46 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: freefuncs.c is never called from anywhere!?]

On Fri, 9 Jun 2000, Tom Lane wrote:

> > Any status on this?
> 
> Nothing done about it yet.
> 
> IIRC, some people were concerned about the fact that freeObject()
> couldn't possibly cope with circular structures, multiply-linked
> subexpressions, etc.  I don't think that's a problem for my intended
> use in the relcache --- the only structures I'll be freeing are ones
> previously read in by the node-reading functions, and those aren't
> going to have any surprises like that.

 IMHO use separate memory context will better and more fast way than 
freeObject(). I use this method in my suggested query cache and in the 
SPI (in SPI_freeplan()) and it is very good (without potential leaks).

All in backend/nodes are (IMHO) very dificult keep up and recursion is
and not gratis feature too. (Homework: write PG in Fortran :-) 

						Karel


Fri, 9 Jun 2000 19:19:15 +0200 (CEST)
Karel Zak <zakkr@zf.jcu.cz>
Re: freefuncs.c is never called from anywhere!? 
Karel Zak <zakkr@zf.jcu.cz> writes:
>> IIRC, some people were concerned about the fact that freeObject()
>> couldn't possibly cope with circular structures, multiply-linked
>> subexpressions, etc.  I don't think that's a problem for my intended
>> use in the relcache --- the only structures I'll be freeing are ones
>> previously read in by the node-reading functions, and those aren't
>> going to have any surprises like that.

>  IMHO use separate memory context will better and more fast way than 
> freeObject().

A separate memory context for each relcache entry?  I don't think so...
contexts aren't likely to be *that* cheap.  Especially since I'd
probably need at least two contexts per relcache entry in order to do it
that way.

			regards, tom lane

Fri, 09 Jun 2000 13:34:43 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: freefuncs.c is never called from anywhere!? 
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> It's tough to maintain clear module APIs with tools like
>> that second-guessing the author's intentions.

> Also, I don't touch the interfaces, only the backend code.  There aren't
> many api stuff there, except spi, which I also don't touch.

I was talking about intermodule APIs within the backend...

			regards, tom lane

Fri, 09 Jun 2000 13:40:20 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: freefuncs.c is never called from anywhere!?] 
> Karel Zak <zakkr@zf.jcu.cz> writes:
> >> IIRC, some people were concerned about the fact that freeObject()
> >> couldn't possibly cope with circular structures, multiply-linked
> >> subexpressions, etc.  I don't think that's a problem for my intended
> >> use in the relcache --- the only structures I'll be freeing are ones
> >> previously read in by the node-reading functions, and those aren't
> >> going to have any surprises like that.
> 
> >  IMHO use separate memory context will better and more fast way than 
> > freeObject().
> 
> A separate memory context for each relcache entry?  I don't think so...

 No. I mean _common_ for manipulation with query/plan tree.



Fri, 9 Jun 2000 20:14:14 +0200 (CEST)
Karel Zak <zakkr@zf.jcu.cz>
Re: freefuncs.c is never called from anywhere!? 
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> It's tough to maintain clear module APIs with tools like
> >> that second-guessing the author's intentions.
> 
> > Also, I don't touch the interfaces, only the backend code.  There aren't
> > many api stuff there, except spi, which I also don't touch.
> 
> I was talking about intermodule APIs within the backend...

Can you give an example?  You mean a function that is meant to be
exported to other modules in the backend, but is not called for some
reason should not be marked as static?

Yes, that is an issue, but reducing our code size and marking functions
static/NOT_USED seems to be a larger benfit.  Of course, IMHO.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 9 Jun 2000 14:39:19 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: freefuncs.c is never called from anywhere!?]