On Wed, Dec 02, 2020 at 02:26:24PM +0100, Peter Eisentraut wrote: > There are a number of elog(LOG) calls that appear to be user-facing, so they > should be ereport()s.
> @@ -8591,25 +8604,46 @@ LogCheckpointEnd(bool restartpoint) > CheckpointStats.ckpt_sync_rels; > average_msecs = (long) ((average_sync_time + 999) / 1000); > > - elog(LOG, "%s complete: wrote %d buffers (%.1f%%); " > - "%d WAL file(s) added, %d removed, %d recycled; " > - "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; " > - "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; " > - "distance=%d kB, estimate=%d kB", +1 to this change. > @@ -1763,7 +1771,8 @@ pq_setkeepalivesidle(int idle, Port *port) > #else > if (idle != 0) > { > - elog(LOG, "setting the keepalive idle time is not supported"); > + ereport(LOG, > + (errmsg("setting the keepalive idle time is not > supported"))); +1 > --- a/src/backend/optimizer/geqo/geqo_erx.c > +++ b/src/backend/optimizer/geqo/geqo_erx.c > @@ -420,7 +420,8 @@ edge_failure(PlannerInfo *root, Gene *gene, int index, > Edge *edge_table, int num > } > } > > - elog(LOG, "no edge found via random decision and total_edges == > 4"); > + ereport(LOG, > + (errmsg("no edge found via random decision and > total_edges == 4"))); The user can't act upon this without reading the source code. This and the other messages proposed in this file are better as elog(). > @@ -343,7 +346,8 @@ PGSharedMemoryCreate(Size size, > * care. > */ > if (!CloseHandle(hmap)) > - elog(LOG, "could not close handle to shared memory: error code > %lu", GetLastError()); > + ereport(LOG, > + (errmsg("could not close handle to shared > memory: error code %lu", GetLastError()))); The numerous messages proposed in src/backend/port/ files are can't-happen events, and there's little a DBA can do without reading the source code. They're better as elog(). > @@ -6108,8 +6111,9 @@ backend_read_statsfile(void) > /* Copy because timestamptz_to_str returns a > static buffer */ > filetime = pstrdup(timestamptz_to_str(file_ts)); > mytime = pstrdup(timestamptz_to_str(cur_ts)); > - elog(LOG, "stats collector's time %s is later > than backend local time %s", > - filetime, mytime); > + ereport(LOG, > + (errmsg("statistics collector's > time %s is later than backend local time %s", > + filetime, > mytime))); +1 > @@ -769,10 +769,11 @@ StartupReplicationOrigin(void) > replication_states[last_state].remote_lsn = > disk_state.remote_lsn; > last_state++; > > - elog(LOG, "recovered replication state of node %u to %X/%X", > - disk_state.roident, > - (uint32) (disk_state.remote_lsn >> 32), > - (uint32) disk_state.remote_lsn); > + ereport(LOG, > + (errmsg("recovered replication state of node %u > to %X/%X", > + disk_state.roident, > + (uint32) (disk_state.remote_lsn > >> 32), > + (uint32) > disk_state.remote_lsn))); +1 > @@ -1914,7 +1914,8 @@ FileClose(File file) > > /* in any case do the unlink */ > if (unlink(vfdP->fileName)) > - elog(LOG, "could not unlink file \"%s\": %m", > vfdP->fileName); > + ereport(LOG, > + (errmsg("could not unlink file \"%s\": > %m", vfdP->fileName))); +1 > > /* and last report the stat results */ > if (stat_errno == 0) > @@ -1922,7 +1923,8 @@ FileClose(File file) > else > { > errno = stat_errno; > - elog(LOG, "could not stat file \"%s\": %m", > vfdP->fileName); > + ereport(LOG, > + (errmsg("could not stat file \"%s\": > %m", vfdP->fileName))); +1 For the changes I didn't mention explicitly (most of them), I'm -0.5. Many of them "can't happen", use source code terms of art, and/or breach guideline "Avoid mentioning called function names, either; instead say what the code was trying to do" (https://www.postgresql.org/docs/devel/error-style-guide.html).