| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Joe Conway <mail(at)joeconway(dot)com> |
| Cc: | pgsql-hackers(at)postgreSQL(dot)org |
| Subject: | Badly designed error reporting code in controldata_utils.c |
| Date: | 2016-03-06 21:24:14 |
| Message-ID: | 32292.1457299454@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
My attention was drawn to the log_error() stuff in controldata_utils.c by
the fact that buildfarm member pademelon spit up on it. The reason for
that compile failure is that pademelon's dinosaur of a compiler doesn't
support __VA_ARGS__. I do not feel a need to get into a discussion about
whether we should move our portability goalposts for the convenience of
this commit, because there are other reasons why this is a crummy solution
for error reporting:
* It uses elog() not ereport() for what seems a not-particularly-internal
error, which among other things means that an entirely inappropriate
errcode() will be reported.
* It relies on strerror(errno), not %m, which may not work reliably even
in elog() and certainly won't in ereport() (because of order-of-evaluation
uncertainties).
* Translatability of the error message in the frontend context seems
a bit dubious; generally we let translators work with the whole string
to be printed, not just part of it.
* It's randomly unlike every single other place we've addressed the
same problem. Everywhere else in src/common does it like this:
#ifndef FRONTEND
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("out of memory")));
#else
fprintf(stderr, _("out of memory\n"));
exit(EXIT_FAILURE);
#endif
and I think that's what this needs to do too, especially in view of the
fact that there are only two places that would have to be fixed anyway.
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joe Conway | 2016-03-06 21:26:34 | Re: Badly designed error reporting code in controldata_utils.c |
| Previous Message | Peter Geoghegan | 2016-03-06 20:25:55 | Re: The plan for FDW-based sharding |