To: Peter Eisentraut <peter_e@gmx.net>
Cc: ljb <lbayuk@mindspring.com>, pgsql-interfaces@postgresql.org
Subject: Re: FE/BE Protocol 3.0 Draft - Some Comments 
In-reply-to: <Pine.LNX.4.44.0304231107520.1676-100000@peter.localdomain> 
References: <Pine.LNX.4.44.0304231107520.1676-100000@peter.localdomain>
Comments: In-reply-to Peter Eisentraut <peter_e@gmx.net>
	message dated "Wed, 23 Apr 2003 16:23:15 +0200"
Date: Wed, 23 Apr 2003 11:26:26 -0400
Message-ID: <26659.1051111586@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>

Peter Eisentraut <peter_e@gmx.net> writes:
> Message + hint is OK, but can you give an example of a "detail"?

Basically I see the 'detail' field as a safety valve for cases where you
otherwise have to compromise between having a reasonably short primary
error message and including all the info that might be useful.

For example, xlog.c's

	elog(emode, "ReadRecord: out-of-sequence SUI %u (after %u) in log file %u, segment %u, offset %u",

might be better rendered

	ereport(emode,
		errmsg("out-of-sequence SUI in xlog page header"),
		errdetail("SUI %u appeared after %u in log file %u, segment %u, offset %u", ...));

The same file also has a bunch of messages along the lines of

        elog(PANIC,
             "The database cluster was initialized with NAMEDATALEN %d,\n"
             "\tbut the backend was compiled with NAMEDATALEN %d.\n"
             "\tIt looks like you need to recompile or initdb.",
             ControlFile->nameDataLen, NAMEDATALEN);

While the advice to re-initdb is clearly a hint, I'm not as sure what to
do with the info about exactly what was found in pg_control.  It might
be best to reword all of these to use a common primary error message,
viz

	ereport(PANIC,
		errmsg("Database cluster is not compatible with server compilation options"),
		errdetail("Cluster has NAMEDATALEN %d, server was compiled with NAMEDATALEN %d", ...),
		errhint("You need to recompile or initdb"));

A point worth making is that with this approach, the translator has
the option to translate just the one shared primary error message,
and not bother with the half-dozen slightly different detail messages,
and still have done a creditable job.  I don't know how much info the
gettext tools can provide about the context in which they found each
string extracted from the source, but it strikes me that it'd be real
handy to label strings as primary message, detail, or hint, so that
translators would know which ones to put priority on translating.

Here's another example that cries out to be broken down into primary
message and detail:

                elog(ERROR, "COPY command, running in backend with "
                     "effective uid %d, could not open file '%s' for "
                     "reading.  Errno = %s (%d).",
                     (int) geteuid(), filename, strerror(errno), errno);

There's no hint there --- it's all factual --- but it seems a tad on
the verbose side.

Same here:

                elog(ERROR, "Can't add class \"%s\" as default for type %s"
                     "\n\tclass \"%s\" already is the default",
                     opcname,
                     TypeNameToString(stmt->datatype),
                     NameStr(opclass->opcname));

The part after \n\t isn't a hint, so it's detail; or we try to claim
that it's part of the primary error message, which is awkward; or we
drop the information, which is bad.

Or here:

            elog(ERROR, "CREATE SCHEMA: permission denied"
                 "\n\t\"%s\" is not a superuser, so cannot create a schema for \"%s\"",
                 owner_name, authId);

Or here:

            elog(FATAL, "pg_nofile: insufficient file descriptors available to start backend.\n"
                 "\tSystem allows %ld, we need at least %d.",
                 no_files, RESERVE_FOR_LD + FD_MINFREE);

I think that without a provision for a detail field, the notion that
primary error messages should fit on one line is going to be widely
ignored.  We don't actually have to have it, but it will encourage good
error-message style.  IMHO anyway.

			regards, tom lane