Thread: RFC for more header changes

RFC for more header changes

From
Alvaro Herrera
Date:
I noticed that "typedef Relation *RelationData" could be moved to
relcache.h instead of rel.h.  This means not #including rel.h in a ton
of header files, instead including just relcache.h.  This is a good
thing because relcache.h is a much more lightweight header; rel.h on the
other hand needs quite a few headers included in order to compile.  All
in all it makes plenty of sense; Relations, as opaque objects, always
come out of the relcache.  There are only a few source files that
actually need access to the struct definition, and thus rel.h becomes a
much less widely used header.  (Also, currently relcache.h includes
rel.h, but in all reality the other way around makes more sense.  This
is what my patch does.)

So that's a first proposal.  Are there objections to that?


While doing that, I noticed that there are some trigger definitions in
rel.h that really belong to trigger.h.  Now that rel.h is not widely
used, it's not too problematic to include trigger.h on it too.  So I
tried to move it there, but the problem I found is that that change
makes trigger.h need execnodes.h and execnodes.h need trigger.h.  As a
whole the situation does not make a lot of sense, and causes those
headers to not compile at all.  To solve the problem I propose, in
addition to the changes mentioned above, to create a new header, say
executor/exectrigger.h, which carries all the function declarations
involving executor stuff (ExecBSInsertTrigger et al).  It would be
possible to also move the code from commands/trigger.c into
executor/exectrigger.c but I think that would be overkill.

(There's a similar analysis to be done for LockRelId and LockInfo, which
cause lmgr.h to need rel.h without much other point.)

Thoughts?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: RFC for more header changes

From
Tom Lane
Date:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> I noticed that "typedef Relation *RelationData" could be moved to
> relcache.h instead of rel.h.

This seems OK.

> While doing that, I noticed that there are some trigger definitions in
> rel.h that really belong to trigger.h.

I think you should leave this alone (and the lock thing as well).
Your proposed solution is uglier than the way it is now.
        regards, tom lane