Re: includedir_internal headers are not self-contained

On Mon, Apr 28, 2014 at 1:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Apr 28, 2014 at 12:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> As far as pg_xlogdump goes, I agree that symbolic fork names are probably
>>> nice, but I think the case for printing db/ts/rel OIDs as a pathname is a
>>> whole lot weaker --- to my taste, that's actually an anti-feature.
>
>> I might be missing something, but, why?
>
> It's more verbose, it's not actually any more information, and in many
> cases it's actively misleading, because what's printed is NOT the real
> file name --- it omits segment numbers for instance.  As a particularly
> egregious example, in xact_desc_commit() we print a pathname including
> MAIN_FORKNUM, which is a flat out lie to the reader, because what will
> actually get deleted is all forks.

Yeah, technically it's a lie, but ls <copy-and-paste-here>* is pretty
handy.  If you format it some other way it's annoying to reformat it.

> So yeah, it's an anti-feature.
>
> BTW, for the same reasons I'm less than impressed with the uses of
> relpath in error messages in, eg, reorderbuffer.c:
>
>                     relation = RelationIdGetRelation(reloid);
>
>                     if (relation == NULL)
>                         elog(ERROR, "could open relation descriptor %s",
>                              relpathperm(change->data.tp.relnode, MAIN_FORKNUM));
>
> Printing anything other than the relation OID here is irrelevant,
> misleading, and inconsistent with our practice everywhere else.
> Let's not even mention the missing "not" in the message text.

Uggh.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company