Another refactoring proposal: move stuff into nodes/nodeFuncs.[ch]

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Another refactoring proposal: move stuff into nodes/nodeFuncs.[ch]
Date: 2008-08-25 15:24:23
Message-ID: 21039.1219677863@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

So I was starting to implement an exprLocation() function according to
previous discussion
http://archives.postgresql.org/pgsql-hackers/2008-08/msg01131.php
and I wondered where to put it. One idea is next to exprType() in
parser/parse_expr.c, but that seems a bit unsatisfactory because it's
likely to eventually be used in many subsystems besides parser/.
So this resurrected a bee that's been in my bonnet for awhile: the
backend has a number of widely-used functions that provide general-purpose
functionality on node trees, but are scattered around in random places
depending on where the need arose first. Some examples are
exprType()
exprTypmod()
expression_tree_walker()
expression_tree_mutator()
query_tree_walker()
query_tree_mutator()
ISTM all of these belong in a central location under backend/nodes.
You could make weaker cases for some other functions like
contain_vars_of_level(), but these ones are definitely widely used
and pretty general-purpose.

The advantages of doing this would be (a) reduce the number of places
to look in when implementing a new node type; (b) eliminate some
cross-subsystem #inclusions that weaken modularity of the backend.

What I'm thinking of doing is migrating these functions into
nodes/nodeFuncs.h and nodes/nodeFuncs.c, which are existing files that
contain almost nothing useful (I think the functions that are there
now are dead or nearly so).

Any objections? Any nominees for additional functions to put there?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-08-25 15:29:50 Re: temporary statistics option at initdb time
Previous Message Magnus Hagander 2008-08-25 15:10:54 Re: temporary statistics option at initdb time