diff options
author | Peter Eisentraut <peter@eisentraut.org> | 2019-04-01 14:24:37 +0200 |
---|---|---|
committer | Peter Eisentraut <peter@eisentraut.org> | 2019-04-01 20:01:35 +0200 |
commit | cc8d41511721d25d557fc02a46c053c0a602fed0 (patch) | |
tree | d2f92acac085be1b9cc4756260c7a4f83d1b0041 /src/bin/psql/command.c | |
parent | b4cc19ab01ffe6a72a915b21aa41536de80923f5 (diff) | |
download | postgresql-cc8d41511721d25d557fc02a46c053c0a602fed0.tar.gz postgresql-cc8d41511721d25d557fc02a46c053c0a602fed0.zip |
Unified logging system for command-line programs
This unifies the various ad hoc logging (message printing, error
printing) systems used throughout the command-line programs.
Features:
- Program name is automatically prefixed.
- Message string does not end with newline. This removes a common
source of inconsistencies and omissions.
- Additionally, a final newline is automatically stripped, simplifying
use of PQerrorMessage() etc., another common source of mistakes.
- I converted error message strings to use %m where possible.
- As a result of the above several points, more translatable message
strings can be shared between different components and between
frontends and backend, without gratuitous punctuation or whitespace
differences.
- There is support for setting a "log level". This is not meant to be
user-facing, but can be used internally to implement debug or
verbose modes.
- Lazy argument evaluation, so no significant overhead if logging at
some level is disabled.
- Some color in the messages, similar to gcc and clang. Set
PG_COLOR=auto to try it out. Some colors are predefined, but can be
customized by setting PG_COLORS.
- Common files (common/, fe_utils/, etc.) can handle logging much more
simply by just using one API without worrying too much about the
context of the calling program, requiring callbacks, or having to
pass "progname" around everywhere.
- Some programs called setvbuf() to make sure that stderr is
unbuffered, even on Windows. But not all programs did that. This
is now done centrally.
Soft goals:
- Reduces vertical space use and visual complexity of error reporting
in the source code.
- Encourages more deliberate classification of messages. For example,
in some cases it wasn't clear without analyzing the surrounding code
whether a message was meant as an error or just an info.
- Concepts and terms are vaguely aligned with popular logging
frameworks such as log4j and Python logging.
This is all just about printing stuff out. Nothing affects program
flow (e.g., fatal exits). The uses are just too varied to do that.
Some existing code had wrappers that do some kind of print-and-exit,
and I adapted those.
I tried to keep the output mostly the same, but there is a lot of
historical baggage to unwind and special cases to consider, and I
might not always have succeeded. One significant change is that
pg_rewind used to write all error messages to stdout. That is now
changed to stderr.
Reviewed-by: Donald Dong <xdong@csumb.edu>
Reviewed-by: Arthur Zakirov <a.zakirov@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/6a609b43-4f57-7348-6480-bd022f924310@2ndquadrant.com
Diffstat (limited to 'src/bin/psql/command.c')
-rw-r--r-- | src/bin/psql/command.c | 157 |
1 files changed, 81 insertions, 76 deletions
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index ab259c473a9..72188b7f3ef 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -28,6 +28,7 @@ #include "libpq-fe.h" #include "pqexpbuffer.h" +#include "fe_utils/logging.h" #include "fe_utils/string_utils.h" #include "common.h" @@ -216,10 +217,9 @@ HandleSlashCmds(PsqlScanState scan_state, if (status == PSQL_CMD_UNKNOWN) { + pg_log_error("invalid command \\%s", cmd); if (pset.cur_cmd_interactive) - psql_error("Invalid command \\%s. Try \\? for help.\n", cmd); - else - psql_error("invalid command \\%s\n", cmd); + pg_log_info("Try \\? for help."); status = PSQL_CMD_ERROR; } @@ -237,7 +237,7 @@ HandleSlashCmds(PsqlScanState scan_state, OT_NORMAL, NULL, false))) { if (active_branch) - psql_error("\\%s: extra argument \"%s\" ignored\n", cmd, arg); + pg_log_warning("\\%s: extra argument \"%s\" ignored", cmd, arg); free(arg); } conditional_stack_pop(cstack); @@ -289,7 +289,7 @@ exec_command(const char *cmd, if (pset.cur_cmd_interactive && !active_branch && !is_branching_command(cmd)) { - psql_error("\\%s command ignored; use \\endif or Ctrl-C to exit current \\if block\n", + pg_log_warning("\\%s command ignored; use \\endif or Ctrl-C to exit current \\if block", cmd); } @@ -549,7 +549,7 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd) pw = getpwuid(user_id); if (!pw) { - psql_error("could not get home directory for user ID %ld: %s\n", + pg_log_error("could not get home directory for user ID %ld: %s", (long) user_id, errno ? strerror(errno) : _("user does not exist")); exit(EXIT_FAILURE); @@ -567,8 +567,8 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd) if (chdir(dir) == -1) { - psql_error("\\%s: could not change directory to \"%s\": %s\n", - cmd, dir, strerror(errno)); + pg_log_error("\\%s: could not change directory to \"%s\": %m", + cmd, dir); success = false; } @@ -908,7 +908,7 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch, { if (!query_buf) { - psql_error("no query buffer\n"); + pg_log_error("no query buffer"); status = PSQL_CMD_ERROR; } else @@ -941,7 +941,7 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch, lineno = atoi(ln); if (lineno < 1) { - psql_error("invalid line number: %s\n", ln); + pg_log_error("invalid line number: %s", ln); status = PSQL_CMD_ERROR; } } @@ -995,16 +995,16 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch, formatPGVersionNumber(pset.sversion, false, sverbuf, sizeof(sverbuf)); if (is_func) - psql_error("The server (version %s) does not support editing function source.\n", + pg_log_error("The server (version %s) does not support editing function source.", sverbuf); else - psql_error("The server (version %s) does not support editing view definitions.\n", + pg_log_error("The server (version %s) does not support editing view definitions.", sverbuf); status = PSQL_CMD_ERROR; } else if (!query_buf) { - psql_error("no query buffer\n"); + pg_log_error("no query buffer"); status = PSQL_CMD_ERROR; } else @@ -1157,7 +1157,7 @@ exec_command_encoding(PsqlScanState scan_state, bool active_branch) { /* set encoding */ if (PQsetClientEncoding(pset.db, encoding) == -1) - psql_error("%s: invalid encoding name or conversion procedure not found\n", encoding); + pg_log_error("%s: invalid encoding name or conversion procedure not found", encoding); else { /* save encoding info into psql internal data */ @@ -1192,7 +1192,7 @@ exec_command_errverbose(PsqlScanState scan_state, bool active_branch) PQSHOW_CONTEXT_ALWAYS); if (msg) { - psql_error("%s", msg); + pg_log_error("%s", msg); PQfreemem(msg); } else @@ -1387,7 +1387,7 @@ exec_command_include(PsqlScanState scan_state, bool active_branch, const char *c if (!fname) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else @@ -1518,12 +1518,12 @@ exec_command_elif(PsqlScanState scan_state, ConditionalStack cstack, break; case IFSTATE_ELSE_TRUE: case IFSTATE_ELSE_FALSE: - psql_error("\\elif: cannot occur after \\else\n"); + pg_log_error("\\elif: cannot occur after \\else"); success = false; break; case IFSTATE_NONE: /* no \if to elif from */ - psql_error("\\elif: no matching \\if\n"); + pg_log_error("\\elif: no matching \\if"); success = false; break; } @@ -1587,12 +1587,12 @@ exec_command_else(PsqlScanState scan_state, ConditionalStack cstack, break; case IFSTATE_ELSE_TRUE: case IFSTATE_ELSE_FALSE: - psql_error("\\else: cannot occur after \\else\n"); + pg_log_error("\\else: cannot occur after \\else"); success = false; break; case IFSTATE_NONE: /* no \if to else from */ - psql_error("\\else: no matching \\if\n"); + pg_log_error("\\else: no matching \\if"); success = false; break; } @@ -1632,7 +1632,7 @@ exec_command_endif(PsqlScanState scan_state, ConditionalStack cstack, break; case IFSTATE_NONE: /* no \if to end */ - psql_error("\\endif: no matching \\if\n"); + pg_log_error("\\endif: no matching \\if"); success = false; break; } @@ -1692,7 +1692,7 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) { if (!opt2) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else @@ -1706,7 +1706,7 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) { if (!opt1) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else @@ -1723,7 +1723,7 @@ exec_command_lo(PsqlScanState scan_state, bool active_branch, const char *cmd) { if (!opt1) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else @@ -1814,7 +1814,7 @@ exec_command_password(PsqlScanState scan_state, bool active_branch) if (strcmp(pw1, pw2) != 0) { - psql_error("Passwords didn't match.\n"); + pg_log_error("Passwords didn't match."); success = false; } else @@ -1831,7 +1831,7 @@ exec_command_password(PsqlScanState scan_state, bool active_branch) if (!encrypted_password) { - psql_error("%s", PQerrorMessage(pset.db)); + pg_log_info("%s", PQerrorMessage(pset.db)); success = false; } else @@ -1883,7 +1883,7 @@ exec_command_prompt(PsqlScanState scan_state, bool active_branch, if (!arg1) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else @@ -1913,7 +1913,7 @@ exec_command_prompt(PsqlScanState scan_state, bool active_branch, result = gets_fromFile(stdin); if (!result) { - psql_error("\\%s: could not read value for variable\n", + pg_log_error("\\%s: could not read value for variable", cmd); success = false; } @@ -2120,12 +2120,12 @@ exec_command_setenv(PsqlScanState scan_state, bool active_branch, if (!envvar) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else if (strchr(envvar, '=') != NULL) { - psql_error("\\%s: environment variable name must not contain \"=\"\n", + pg_log_error("\\%s: environment variable name must not contain \"=\"", cmd); success = false; } @@ -2186,19 +2186,19 @@ exec_command_sf_sv(PsqlScanState scan_state, bool active_branch, formatPGVersionNumber(pset.sversion, false, sverbuf, sizeof(sverbuf)); if (is_func) - psql_error("The server (version %s) does not support showing function source.\n", + pg_log_error("The server (version %s) does not support showing function source.", sverbuf); else - psql_error("The server (version %s) does not support showing view definitions.\n", + pg_log_error("The server (version %s) does not support showing view definitions.", sverbuf); status = PSQL_CMD_ERROR; } else if (!obj_desc) { if (is_func) - psql_error("function name is required\n"); + pg_log_error("function name is required"); else - psql_error("view name is required\n"); + pg_log_error("view name is required"); status = PSQL_CMD_ERROR; } else if (!lookup_object_oid(eot, obj_desc, &obj_oid)) @@ -2356,7 +2356,7 @@ exec_command_unset(PsqlScanState scan_state, bool active_branch, if (!opt) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); success = false; } else if (!SetVariable(pset.vars, opt, NULL)) @@ -2389,14 +2389,14 @@ exec_command_write(PsqlScanState scan_state, bool active_branch, if (!query_buf) { - psql_error("no query buffer\n"); + pg_log_error("no query buffer"); status = PSQL_CMD_ERROR; } else { if (!fname) { - psql_error("\\%s: missing required argument\n", cmd); + pg_log_error("\\%s: missing required argument", cmd); status = PSQL_CMD_ERROR; } else @@ -2415,7 +2415,7 @@ exec_command_write(PsqlScanState scan_state, bool active_branch, } if (!fd) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); status = PSQL_CMD_ERROR; } } @@ -2443,7 +2443,7 @@ exec_command_write(PsqlScanState scan_state, bool active_branch, if (result == EOF) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); status = PSQL_CMD_ERROR; } } @@ -2883,8 +2883,8 @@ do_connect(enum trivalue reuse_previous_specification, * connect to the wrong database by using defaults, so require all * parameters to be specified. */ - psql_error("All connection parameters must be supplied because no " - "database connection exists\n"); + pg_log_error("All connection parameters must be supplied because no " + "database connection exists"); return false; } @@ -3067,15 +3067,15 @@ do_connect(enum trivalue reuse_previous_specification, */ if (pset.cur_cmd_interactive) { - psql_error("%s", PQerrorMessage(n_conn)); + pg_log_info("%s", PQerrorMessage(n_conn)); /* pset.db is left unmodified */ if (o_conn) - psql_error("Previous connection kept\n"); + pg_log_info("Previous connection kept"); } else { - psql_error("\\connect: %s", PQerrorMessage(n_conn)); + pg_log_error("\\connect: %s", PQerrorMessage(n_conn)); if (o_conn) { PQfinish(o_conn); @@ -3337,7 +3337,7 @@ editFile(const char *fname, int lineno) #endif if (!editor_lineno_arg) { - psql_error("environment variable PSQL_EDITOR_LINENUMBER_ARG must be set to specify a line number\n"); + pg_log_error("environment variable PSQL_EDITOR_LINENUMBER_ARG must be set to specify a line number"); return false; } } @@ -3366,9 +3366,9 @@ editFile(const char *fname, int lineno) #endif result = system(sys); if (result == -1) - psql_error("could not start editor \"%s\"\n", editorName); + pg_log_error("could not start editor \"%s\"", editorName); else if (result == 127) - psql_error("could not start /bin/sh\n"); + pg_log_error("could not start /bin/sh"); free(sys); return result == 0; @@ -3406,7 +3406,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, ret = GetTempPath(MAXPGPATH, tmpdir); if (ret == 0 || ret > MAXPGPATH) { - psql_error("could not locate temporary directory: %s\n", + pg_log_error("could not locate temporary directory: %s", !ret ? strerror(errno) : ""); return false; } @@ -3433,7 +3433,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (fd == -1 || !stream) { - psql_error("could not open temporary file \"%s\": %s\n", fname, strerror(errno)); + pg_log_error("could not open temporary file \"%s\": %m", fname); error = true; } else @@ -3448,21 +3448,21 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (fwrite(query_buf->data, 1, ql, stream) != ql) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); if (fclose(stream) != 0) - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); if (remove(fname) != 0) - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } else if (fclose(stream) != 0) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); if (remove(fname) != 0) - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } } @@ -3470,7 +3470,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (!error && stat(fname, &before) != 0) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } @@ -3480,7 +3480,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (!error && stat(fname, &after) != 0) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } @@ -3489,7 +3489,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, stream = fopen(fname, PG_BINARY_R); if (!stream) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } else @@ -3503,7 +3503,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (ferror(stream)) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } else if (edited) @@ -3520,7 +3520,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, { if (remove(fname) == -1) { - psql_error("%s: %s\n", fname, strerror(errno)); + pg_log_error("%s: %m", fname); error = true; } } @@ -3578,7 +3578,7 @@ process_file(char *filename, bool use_relative_path) if (!fd) { - psql_error("%s: %s\n", filename, strerror(errno)); + pg_log_error("%s: %m", filename); return EXIT_FAILURE; } } @@ -3591,12 +3591,17 @@ process_file(char *filename, bool use_relative_path) oldfilename = pset.inputfile; pset.inputfile = filename; + pg_logging_config(pset.inputfile ? 0 : PG_LOG_FLAG_TERSE); + result = MainLoop(fd); if (fd != stdin) fclose(fd); pset.inputfile = oldfilename; + + pg_logging_config(pset.inputfile ? 0 : PG_LOG_FLAG_TERSE); + return result; } @@ -3721,7 +3726,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) match_pos = i; else { - psql_error("\\pset: ambiguous abbreviation \"%s\" matches both \"%s\" and \"%s\"\n", + pg_log_error("\\pset: ambiguous abbreviation \"%s\" matches both \"%s\" and \"%s\"", value, formats[match_pos].name, formats[i].name); return false; @@ -3741,7 +3746,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) } else { - psql_error("\\pset: allowed formats are aligned, asciidoc, csv, html, latex, latex-longtable, troff-ms, unaligned, wrapped\n"); + pg_log_error("\\pset: allowed formats are aligned, asciidoc, csv, html, latex, latex-longtable, troff-ms, unaligned, wrapped"); return false; } } @@ -3760,7 +3765,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) popt->topt.line_style = &pg_utf8format; else { - psql_error("\\pset: allowed line styles are ascii, old-ascii, unicode\n"); + pg_log_error("\\pset: allowed line styles are ascii, old-ascii, unicode"); return false; } } @@ -3775,7 +3780,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) refresh_utf8format(&(popt->topt)); else { - psql_error("\\pset: allowed Unicode border line styles are single, double\n"); + pg_log_error("\\pset: allowed Unicode border line styles are single, double"); return false; } } @@ -3790,7 +3795,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) refresh_utf8format(&(popt->topt)); else { - psql_error("\\pset: allowed Unicode column line styles are single, double\n"); + pg_log_error("\\pset: allowed Unicode column line styles are single, double"); return false; } } @@ -3805,7 +3810,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) refresh_utf8format(&(popt->topt)); else { - psql_error("\\pset: allowed Unicode header line styles are single, double\n"); + pg_log_error("\\pset: allowed Unicode header line styles are single, double"); return false; } } @@ -3848,12 +3853,12 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) /* CSV separator has to be a one-byte character */ if (strlen(value) != 1) { - psql_error("\\pset: csv_fieldsep must be a single one-byte character\n"); + pg_log_error("\\pset: csv_fieldsep must be a single one-byte character"); return false; } if (value[0] == '"' || value[0] == '\n' || value[0] == '\r') { - psql_error("\\pset: csv_fieldsep cannot be a double quote, a newline, or a carriage return\n"); + pg_log_error("\\pset: csv_fieldsep cannot be a double quote, a newline, or a carriage return"); return false; } popt->topt.csvFieldSep[0] = value[0]; @@ -3990,7 +3995,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) } else { - psql_error("\\pset: unknown option: %s\n", param); + pg_log_error("\\pset: unknown option: %s", param); return false; } @@ -4176,7 +4181,7 @@ printPsetInfo(const char *param, struct printQueryOpt *popt) else { - psql_error("\\pset: unknown option: %s\n", param); + pg_log_error("\\pset: unknown option: %s", param); return false; } @@ -4332,7 +4337,7 @@ do_shell(const char *command) if (result == 127 || result == -1) { - psql_error("\\!: failed\n"); + pg_log_error("\\!: failed"); return false; } return true; @@ -4357,7 +4362,7 @@ do_watch(PQExpBuffer query_buf, double sleep) if (!query_buf || query_buf->len <= 0) { - psql_error("\\watch cannot be used with an empty query\n"); + pg_log_error("\\watch cannot be used with an empty query"); return false; } @@ -4654,7 +4659,7 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid, appendPQExpBufferStr(buf, "CREATE OR REPLACE VIEW "); break; default: - psql_error("\"%s.%s\" is not a view\n", + pg_log_error("\"%s.%s\" is not a view", nspname, relname); result = false; break; @@ -4670,7 +4675,7 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid, pset.encoding, standard_strings())) { - psql_error("could not parse reloptions array\n"); + pg_log_error("could not parse reloptions array"); result = false; } appendPQExpBufferChar(buf, ')'); @@ -4759,7 +4764,7 @@ strip_lineno_from_objdesc(char *obj) lineno = atoi(c); if (lineno < 1) { - psql_error("invalid line number: %s\n", c); + pg_log_error("invalid line number: %s", c); return 0; } @@ -4861,7 +4866,7 @@ minimal_error_message(PGresult *res) appendPQExpBufferStr(msg, "(not available)"); appendPQExpBufferChar(msg, '\n'); - psql_error("%s", msg->data); + pg_log_error("%s", msg->data); destroyPQExpBuffer(msg); } |