Plug-in common/logging.h with vacuumlo and oid2name
Hi all,
While refactoring some code, I have bumped into $subject, which causes
both tools to have incorrect error message formats and progname being
used all through the code, sometimes incorrectly or just missing.
At the same time, we are missing a call to set_pglocale_pgservice()
for both binaries.
Any objections to the cleanup attached?
--
Michael
Attachments:
vacuumlo-oid2name-logging.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index d3a4a50005..b33e4d3d82 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -11,6 +11,7 @@
#include "catalog/pg_class_d.h"
+#include "common/logging.h"
#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -85,6 +86,8 @@ get_opts(int argc, char **argv, struct options *my_opts)
const char *progname;
int optindex;
+ pg_logging_init(argv[0]);
+ set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("oid2name"));
progname = get_progname(argv[0]);
/* set the defaults */
@@ -320,8 +323,8 @@ sql_conn(struct options *my_opts)
if (!conn)
{
- fprintf(stderr, "%s: could not connect to database %s\n",
- "oid2name", my_opts->dbname);
+ pg_log_error("could not connect to database %s",
+ my_opts->dbname);
exit(1);
}
@@ -339,8 +342,8 @@ sql_conn(struct options *my_opts)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- fprintf(stderr, "%s: could not connect to database %s: %s",
- "oid2name", my_opts->dbname, PQerrorMessage(conn));
+ pg_log_error("could not connect to database %s: %s",
+ my_opts->dbname, PQerrorMessage(conn));
PQfinish(conn);
exit(1);
}
@@ -348,8 +351,8 @@ sql_conn(struct options *my_opts)
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "oid2name: could not clear search_path: %s\n",
- PQerrorMessage(conn));
+ pg_log_error("could not clear search_path: %s",
+ PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
exit(-1);
@@ -382,8 +385,8 @@ sql_exec(PGconn *conn, const char *todo, bool quiet)
/* check and deal with errors */
if (!res || PQresultStatus(res) > 2)
{
- fprintf(stderr, "oid2name: query failed: %s\n", PQerrorMessage(conn));
- fprintf(stderr, "oid2name: query was: %s\n", todo);
+ pg_log_error("query failed: %s", PQerrorMessage(conn));
+ pg_log_error("query was: %s", todo);
PQclear(res);
PQfinish(conn);
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 73c06a043e..7fbcaee846 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -23,6 +23,7 @@
#include "catalog/pg_class_d.h"
+#include "common/logging.h"
#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -109,8 +110,7 @@ vacuumlo(const char *database, const struct _param *param)
conn = PQconnectdbParams(keywords, values, true);
if (!conn)
{
- fprintf(stderr, "Connection to database \"%s\" failed\n",
- database);
+ pg_log_error("connection to database \"%s\" failed", database);
return -1;
}
@@ -129,8 +129,8 @@ vacuumlo(const char *database, const struct _param *param)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- fprintf(stderr, "Connection to database \"%s\" failed:\n%s",
- database, PQerrorMessage(conn));
+ pg_log_error("connection to database \"%s\" failed: %s",
+ database, PQerrorMessage(conn));
PQfinish(conn);
return -1;
}
@@ -145,8 +145,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "Failed to set search_path:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to set search_path: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -165,8 +164,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to create temp table:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to create temp table: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -182,8 +180,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to vacuum temp table:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to vacuum temp table: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -212,8 +209,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "Failed to find OID columns:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to find OID columns: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -238,7 +234,7 @@ vacuumlo(const char *database, const struct _param *param)
if (!schema || !table || !field)
{
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
if (schema != NULL)
@@ -257,9 +253,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, buf);
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to check %s in table %s.%s:\n",
- field, schema, table);
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to check %s in table %s.%s: %s",
+ field, schema, table, PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -288,8 +283,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, "begin");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to start transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to start transaction: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -302,7 +296,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
+ pg_log_error("DECLARE CURSOR failed: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -319,7 +313,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "FETCH FORWARD failed: %s", PQerrorMessage(conn));
+ pg_log_error("FETCH FORWARD failed: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -347,8 +341,8 @@ vacuumlo(const char *database, const struct _param *param)
{
if (lo_unlink(conn, lo) < 0)
{
- fprintf(stderr, "\nFailed to remove lo %u: ", lo);
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("\nfailed to remove lo %u: %s", lo,
+ PQerrorMessage(conn));
if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
success = false;
@@ -367,8 +361,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, "commit");
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to commit transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to commit transaction: %s",
+ PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -378,8 +372,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, "begin");
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to start transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to start transaction: %s",
+ PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -398,8 +392,8 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, "commit");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to commit transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to commit transaction: %s",
+ PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -471,6 +465,8 @@ main(int argc, char **argv)
const char *progname;
int optindex;
+ pg_logging_init(argv[0]);
+ set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("vacuumlo"));
progname = get_progname(argv[0]);
/* Set default parameter values */
@@ -512,9 +508,7 @@ main(int argc, char **argv)
param.transaction_limit = strtol(optarg, NULL, 10);
if (param.transaction_limit < 0)
{
- fprintf(stderr,
- "%s: transaction limit must not be negative (0 disables)\n",
- progname);
+ pg_log_error("transaction limit must not be negative (0 disables)");
exit(1);
}
break;
@@ -526,7 +520,7 @@ main(int argc, char **argv)
port = strtol(optarg, NULL, 10);
if ((port < 1) || (port > 65535))
{
- fprintf(stderr, "%s: invalid port number: %s\n", progname, optarg);
+ pg_log_error("invalid port number: %s", optarg);
exit(1);
}
param.pg_port = pg_strdup(optarg);
@@ -552,7 +546,7 @@ main(int argc, char **argv)
/* No database given? Show usage */
if (optind >= argc)
{
- fprintf(stderr, "vacuumlo: missing required argument: database name\n");
+ pg_log_error("missing required argument: database name");
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
}
On 2019-08-20 03:28, Michael Paquier wrote:
+ pg_log_error("\nfailed to remove lo %u: %s", lo, + PQerrorMessage(conn));
This newline should be removed.
progname can probably be made into a local variable now.
The rest looks good to me.
Do we need set_pglocale_pgservice() calls here if we're not doing NLS?
Does the logging stuff require it? I'm not sure.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Sep 04, 2019 at 10:17:57AM +0200, Peter Eisentraut wrote:
On 2019-08-20 03:28, Michael Paquier wrote:
+ pg_log_error("\nfailed to remove lo %u: %s", lo, + PQerrorMessage(conn));This newline should be removed.
Thanks, missed that.
progname can probably be made into a local variable now.
Can it? vacuumlo() still uses the progname from _param for the
connection string.
Do we need set_pglocale_pgservice() calls here if we're not doing NLS?
Does the logging stuff require it? I'm not sure.
The logging part does not require it, but this can be used for
PGSYSCONFDIR, no?
--
Michael
On 2019-09-04 14:17, Michael Paquier wrote:
progname can probably be made into a local variable now.
Can it? vacuumlo() still uses the progname from _param for the
connection string.
Yeah, probably best to leave it as is for now.
Do we need set_pglocale_pgservice() calls here if we're not doing NLS?
Does the logging stuff require it? I'm not sure.The logging part does not require it, but this can be used for
PGSYSCONFDIR, no?
How does PGSYSCONFDIR come into play here?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Sep 04, 2019 at 02:37:12PM +0200, Peter Eisentraut wrote:
Do we need set_pglocale_pgservice() calls here if we're not doing NLS?
Does the logging stuff require it? I'm not sure.The logging part does not require it, but this can be used for
PGSYSCONFDIR, no?How does PGSYSCONFDIR come into play here?
There is an argument to allow libpq to find out a service file for
a connection from the executable path. Note that oid2name can use a
connection string for connection, but not vacuumlo, so I somewhat
missed that.
--
Michael
On Thu, Sep 05, 2019 at 08:59:41AM +0900, Michael Paquier wrote:
There is an argument to allow libpq to find out a service file for
a connection from the executable path. Note that oid2name can use a
connection string for connection, but not vacuumlo, so I somewhat
missed that.
If you think that's not worth considering for now, I am fine to drop
that part. What do you think about the updated patch attached then?
--
Michael
Attachments:
vacuumlo-oid2name-logging-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index df4f9c062f..fa1e7959e7 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -11,6 +11,7 @@
#include "catalog/pg_class_d.h"
+#include "common/logging.h"
#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -85,6 +86,7 @@ get_opts(int argc, char **argv, struct options *my_opts)
const char *progname;
int optindex;
+ pg_logging_init(argv[0]);
progname = get_progname(argv[0]);
/* set the defaults */
@@ -328,8 +330,8 @@ sql_conn(struct options *my_opts)
if (!conn)
{
- fprintf(stderr, "%s: could not connect to database %s\n",
- "oid2name", my_opts->dbname);
+ pg_log_error("could not connect to database %s",
+ my_opts->dbname);
exit(1);
}
@@ -347,8 +349,8 @@ sql_conn(struct options *my_opts)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- fprintf(stderr, "%s: could not connect to database %s: %s",
- "oid2name", my_opts->dbname, PQerrorMessage(conn));
+ pg_log_error("could not connect to database %s: %s",
+ my_opts->dbname, PQerrorMessage(conn));
PQfinish(conn);
exit(1);
}
@@ -356,8 +358,8 @@ sql_conn(struct options *my_opts)
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "oid2name: could not clear search_path: %s\n",
- PQerrorMessage(conn));
+ pg_log_error("could not clear search_path: %s",
+ PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
exit(-1);
@@ -390,8 +392,8 @@ sql_exec(PGconn *conn, const char *todo, bool quiet)
/* check and deal with errors */
if (!res || PQresultStatus(res) > 2)
{
- fprintf(stderr, "oid2name: query failed: %s\n", PQerrorMessage(conn));
- fprintf(stderr, "oid2name: query was: %s\n", todo);
+ pg_log_error("query failed: %s", PQerrorMessage(conn));
+ pg_log_error("query was: %s", todo);
PQclear(res);
PQfinish(conn);
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 73c06a043e..533e2ce33c 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -23,6 +23,7 @@
#include "catalog/pg_class_d.h"
+#include "common/logging.h"
#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -109,8 +110,7 @@ vacuumlo(const char *database, const struct _param *param)
conn = PQconnectdbParams(keywords, values, true);
if (!conn)
{
- fprintf(stderr, "Connection to database \"%s\" failed\n",
- database);
+ pg_log_error("connection to database \"%s\" failed", database);
return -1;
}
@@ -129,8 +129,8 @@ vacuumlo(const char *database, const struct _param *param)
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) == CONNECTION_BAD)
{
- fprintf(stderr, "Connection to database \"%s\" failed:\n%s",
- database, PQerrorMessage(conn));
+ pg_log_error("connection to database \"%s\" failed: %s",
+ database, PQerrorMessage(conn));
PQfinish(conn);
return -1;
}
@@ -145,8 +145,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "Failed to set search_path:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to set search_path: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -165,8 +164,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to create temp table:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to create temp table: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -182,8 +180,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to vacuum temp table:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to vacuum temp table: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -212,8 +209,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "Failed to find OID columns:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to find OID columns: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -238,7 +234,7 @@ vacuumlo(const char *database, const struct _param *param)
if (!schema || !table || !field)
{
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("%s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
if (schema != NULL)
@@ -257,9 +253,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, buf);
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to check %s in table %s.%s:\n",
- field, schema, table);
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to check %s in table %s.%s: %s",
+ field, schema, table, PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -288,8 +283,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, "begin");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to start transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to start transaction: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -302,7 +296,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
+ pg_log_error("DECLARE CURSOR failed: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -319,7 +313,7 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, buf);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
- fprintf(stderr, "FETCH FORWARD failed: %s", PQerrorMessage(conn));
+ pg_log_error("FETCH FORWARD failed: %s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -347,8 +341,8 @@ vacuumlo(const char *database, const struct _param *param)
{
if (lo_unlink(conn, lo) < 0)
{
- fprintf(stderr, "\nFailed to remove lo %u: ", lo);
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to remove lo %u: %s", lo,
+ PQerrorMessage(conn));
if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
success = false;
@@ -367,8 +361,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, "commit");
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to commit transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to commit transaction: %s",
+ PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -378,8 +372,8 @@ vacuumlo(const char *database, const struct _param *param)
res2 = PQexec(conn, "begin");
if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to start transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to start transaction: %s",
+ PQerrorMessage(conn));
PQclear(res2);
PQclear(res);
PQfinish(conn);
@@ -398,8 +392,8 @@ vacuumlo(const char *database, const struct _param *param)
res = PQexec(conn, "commit");
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
- fprintf(stderr, "Failed to commit transaction:\n");
- fprintf(stderr, "%s", PQerrorMessage(conn));
+ pg_log_error("failed to commit transaction: %s",
+ PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return -1;
@@ -471,6 +465,7 @@ main(int argc, char **argv)
const char *progname;
int optindex;
+ pg_logging_init(argv[0]);
progname = get_progname(argv[0]);
/* Set default parameter values */
@@ -512,9 +507,7 @@ main(int argc, char **argv)
param.transaction_limit = strtol(optarg, NULL, 10);
if (param.transaction_limit < 0)
{
- fprintf(stderr,
- "%s: transaction limit must not be negative (0 disables)\n",
- progname);
+ pg_log_error("transaction limit must not be negative (0 disables)");
exit(1);
}
break;
@@ -526,7 +519,7 @@ main(int argc, char **argv)
port = strtol(optarg, NULL, 10);
if ((port < 1) || (port > 65535))
{
- fprintf(stderr, "%s: invalid port number: %s\n", progname, optarg);
+ pg_log_error("invalid port number: %s", optarg);
exit(1);
}
param.pg_port = pg_strdup(optarg);
@@ -552,7 +545,7 @@ main(int argc, char **argv)
/* No database given? Show usage */
if (optind >= argc)
{
- fprintf(stderr, "vacuumlo: missing required argument: database name\n");
+ pg_log_error("missing required argument: database name");
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
}
On 2019-09-05 01:59, Michael Paquier wrote:
On Wed, Sep 04, 2019 at 02:37:12PM +0200, Peter Eisentraut wrote:
Do we need set_pglocale_pgservice() calls here if we're not doing NLS?
Does the logging stuff require it? I'm not sure.The logging part does not require it, but this can be used for
PGSYSCONFDIR, no?How does PGSYSCONFDIR come into play here?
There is an argument to allow libpq to find out a service file for
a connection from the executable path. Note that oid2name can use a
connection string for connection, but not vacuumlo, so I somewhat
missed that.
Oh I see what's going on. PGSYSCONFDIR is used by libpq, and
set_pglocale_pgservice() does some path mangling on PGSYSCONFDIR if set
for Windows. Shouldn't libpq do that itself? Worth looking into but
probably unrelated to your patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Patch looks good to me, please push.
Generally speaking I find the 'progname' handling a bit silly (since we
have it both as a variable in each program and also in logging.c
separately), but that's not the fault of this patch, and this patch
doesn't make it worse. That said, I think some other messages in
vacuumlo can be made pg_log_somelevel(), based on occurrences of
'progname'.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Sep 05, 2019 at 09:59:51AM -0400, Alvaro Herrera wrote:
Patch looks good to me, please push.
Thanks, applied without the calls to set_pglocale_pgservice().
Generally speaking I find the 'progname' handling a bit silly (since we
have it both as a variable in each program and also in logging.c
separately), but that's not the fault of this patch, and this patch
doesn't make it worse. That said, I think some other messages in
vacuumlo can be made pg_log_somelevel(), based on occurrences of
'progname'.
That would mean moving some of the messages from stdout to stderr. We
could do that.
--
Michael