I wrote:
> Christoph Berg <cb@df7cb.de> writes:
>> $ grep -r include 9.4/server/common/ | grep \"
>> 9.4/server/common/fe_memutils.h:#include "utils/palloc.h"
>> 9.4/server/common/relpath.h:#include "catalog/catversion.h" /* pgrminclude ignore */
>> 9.4/server/common/relpath.h:#include "storage/relfilenode.h"
> The catversion dependency also seems pretty damn brain-dead in this
> context. Let's see if we can get rid of that. As for relfilenode,
> if we need that in relpath.h maybe the answer is that relfilenode.h
> has to be in common/.
On closer inspection, the issue here is really that putting relpath.h/.c
in common/ was completely misguided from the get-go. It's unnecessary:
there's nothing outside the backend that uses it, except for
contrib/pg_xlogdump which could very easily do without it. And relpath.h
is a serious failure from a modularity standpoint anyway, because there is
code all over the backend that has intimate familiarity with the pathname
construction rules. We could possibly clean that up to the extent of
being able to hide TABLESPACE_VERSION_DIRECTORY inside relpath.c, but what
then? We'd still be talking about having CATALOG_VERSION_NO compiled into
frontend code for any frontend code that actually made use of relpath.c,
which is surely not such a great idea.
So it seems to me the right fix for the relpath end of it is to push most
of relpath.c back where it came from, which I think was backend/catalog/.
There might be some value in keeping the forkname-related code in common/;
that's not quite so intimately tied to the backend version as relpath()
itself. (And indeed forkNames[] is the only thing that pg_xlogdump.c
needs.) But I'm not really convinced that a module encapsulating just the
fork names is worth the trouble, and especially not convinced that
frontend code needs to be dealing with fork names. Thoughts?
regards, tom lane
