On Tue, Mar 02, 2021 at 11:52:33AM +0900, miyake_kouta wrote: > I modified the patch based on other binaries. > In this new patch, if there is a $PGUSER, then it's set to login. > If not, get_user_name_or_exit is excuted. > Plese let me know what you think about this change.
Your patch makes the database selection slightly better, but I think that we can do better and simpler than that. So please see the attached. One thing on HEAD that looks like a bug to me is that if one uses a pgbench command without specifying user, port and/or name in the command for an environment without PGDATABASE, PGPORT and PGHOST set, then the debug log just before doConnect() prints empty strings for all that, which is basically useless so one has no idea where the connection happens. Like any other src/bin/ facilities, let's instead remove all the getenv() calls at the beginning of pgbench.c and let's libpq handle those environment variables if the parameters are NULL (aka in the case of no values given directly in the options). This requires to move the debug log after doConnect(), which is no big deal anyway as a failure results in an exit(1) immediately with a log telling where the connection failed. What do you think? -- Michael
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 31a4df45f5..ab56272f2e 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -32,6 +32,7 @@
#endif
#include "postgres_fe.h"
+#include "common/username.h"
#include <ctype.h>
#include <float.h>
@@ -240,10 +241,10 @@ bool is_connect; /* establish connection for each transaction */
bool report_per_command; /* report per-command latencies */
int main_pid; /* main process id used in log filename */
-char *pghost = "";
-char *pgport = "";
-char *login = NULL;
-char *dbName;
+const char *pghost = NULL;
+const char *pgport = NULL;
+const char *username = NULL;
+const char *dbName = NULL;
char *logfile_prefix = NULL;
const char *progname;
@@ -1191,7 +1192,7 @@ doConnect(void)
keywords[1] = "port";
values[1] = pgport;
keywords[2] = "user";
- values[2] = login;
+ values[2] = username;
keywords[3] = "password";
values[3] = password;
keywords[4] = "dbname";
@@ -5483,13 +5484,6 @@ main(int argc, char **argv)
}
}
- if ((env = getenv("PGHOST")) != NULL && *env != '\0')
- pghost = env;
- if ((env = getenv("PGPORT")) != NULL && *env != '\0')
- pgport = env;
- else if ((env = getenv("PGUSER")) != NULL && *env != '\0')
- login = env;
-
state = (CState *) pg_malloc0(sizeof(CState));
/* set random seed early, because it may be used while parsing scripts. */
@@ -5610,7 +5604,7 @@ main(int argc, char **argv)
}
break;
case 'U':
- login = pg_strdup(optarg);
+ username = pg_strdup(optarg);
break;
case 'l':
benchmarking_option_set = true;
@@ -5860,10 +5854,10 @@ main(int argc, char **argv)
{
if ((env = getenv("PGDATABASE")) != NULL && *env != '\0')
dbName = env;
- else if (login != NULL && *login != '\0')
- dbName = login;
+ else if ((env = getenv("PGUSER")) != NULL && *env != '\0')
+ dbName = env;
else
- dbName = "";
+ dbName = get_user_name_or_exit(progname);
}
if (optind < argc)
@@ -6026,16 +6020,16 @@ main(int argc, char **argv)
initRandomState(&state[i].cs_func_rs);
}
- pg_log_debug("pghost: %s pgport: %s nclients: %d %s: %d dbName: %s",
- pghost, pgport, nclients,
- duration <= 0 ? "nxacts" : "duration",
- duration <= 0 ? nxacts : duration, dbName);
-
/* opening connection... */
con = doConnect();
if (con == NULL)
exit(1);
+ pg_log_debug("pghost: %s pgport: %s nclients: %d %s: %d dbName: %s",
+ PQhost(con), PQport(con), nclients,
+ duration <= 0 ? "nxacts" : "duration",
+ duration <= 0 ? nxacts : duration, PQdb(con));
+
if (internal_script_used)
GetTableInfo(con, scale_given);
signature.asc
Description: PGP signature
