On 04/27/2014 11:33 PM, Tom Lane wrote:
> I wrote:
>> 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/.
>
> In short, what I'm proposing here is to revert commit
> a73018392636ce832b09b5c31f6ad1f18a4643ea, lock stock n barrel.
> According to the commit message, the point of that was to allow
> pg_xlogdump to use relpath(), but I do not see it doing so; and
> if it tried, I don't know where it would get an accurate value of
> CATALOG_VERSION_NO from. So I think that was just poorly thought out.
>
> What contrib/pg_xlogdump *is* using is the forkNames[] array, which
> it uses to print the fork-number field of a BkpBlock symbolically.
> I'm inclined to think that printing numerically is good enough there,
> and a lot more robust.
>
> Comments? If there's anyone who has a really good use-case for using
> relpath() from outside the backend, better speak up.
I'm using it in the pg_rewind tool. It needs to know how to map
relfilenodes to physical files.
It has quite intimate knowledge of the on-disk layout anyway, so it
wouldn't be the end of the world to just copy-paste that code. But would
rather not, of course.
- Heikki