pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Fabien COELHO <coelho@cri.ensmp.fr>
To: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jaime Soler <jaime.soler@gmail.com>
Subject: Re: pgbench and timestamps (bounced)
Date: Thu, 9 Jul 2020 08:41:22 +0200 (CEST)
Message-ID: <alpine.DEB.2.22.394.2007090839210.417896@pseudo> (raw )
In-Reply-To: <alpine.DEB.2.22.394.2006291740420.805678@pseudo >
References: <alpine.DEB.2.22.394.2006291740420.805678@pseudo >
Attached v2 fixes some errors, per cfbot.
--
Fabien.
Attachments:
pgbench-var-fix-2.patch (text/x-diff, 16.9K) download
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 9f3bb5fce6..9894ae9c04 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -998,15 +998,14 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
<para>
There is a simple variable-substitution facility for script files.
Variable names must consist of letters (including non-Latin letters),
- digits, and underscores.
+ digits (but not on the first character), and underscores.
Variables can be set by the command-line <option>-D</option> option,
explained above, or by the meta commands explained below.
In addition to any variables preset by <option>-D</option> command-line options,
there are a few variables that are preset automatically, listed in
<xref linkend="pgbench-automatic-variables"/>. A value specified for these
variables using <option>-D</option> takes precedence over the automatic presets.
- Once set, a variable's
- value can be inserted into a SQL command by writing
+ Once set, a variable's value can be inserted into a SQL command by writing
<literal>:</literal><replaceable>variablename</replaceable>. When running more than
one client session, each session has its own set of variables.
<application>pgbench</application> supports up to 255 variable uses in one
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 08a5947a9e..3879bef6da 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -119,12 +119,24 @@ typedef int pthread_attr_t;
static int pthread_create(pthread_t *thread, pthread_attr_t *attr, void *(*start_routine) (void *), void *arg);
static int pthread_join(pthread_t th, void **thread_return);
+
+typedef HANDLE pthread_mutex_t;
+static void pthread_mutex_init(pthread_mutex_t *pm, void *unused);
+static void pthread_mutex_lock(pthread_mutex_t *pm);
+static void pthread_mutex_unlock(pthread_mutex_t *pm);
+static void pthread_mutex_destroy(pthread_mutex_t *pm);
+
#elif defined(ENABLE_THREAD_SAFETY)
/* Use platform-dependent pthread capability */
#include <pthread.h>
#else
/* No threads implementation, use none (-j 1) */
#define pthread_t void *
+#define pthread_mutex_t void *
+#define pthread_mutex_init(m, p)
+#define pthread_mutex_lock(m)
+#define pthread_mutex_unlock(m)
+#define pthread_mutex_destroy(m)
#endif
@@ -422,7 +434,7 @@ typedef struct
instr_time txn_begin; /* used for measuring schedule lag times */
instr_time stmt_begin; /* used for measuring statement latencies */
- bool prepared[MAX_SCRIPTS]; /* whether client prepared the script */
+ bool* prepared[MAX_SCRIPTS]; /* whether client prepared the script commands */
/* per client collected stats */
int64 cnt; /* client transaction count, for -t */
@@ -472,7 +484,7 @@ typedef struct
*/
#define MAX_ARGS 256
-typedef enum MetaCommand
+typedef enum Meta
{
META_NONE, /* not a known meta-command */
META_SET, /* \set */
@@ -503,6 +515,8 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
*
* lines The raw, possibly multi-line command text. Variable substitution
* not applied.
+ * substituted Whether command :-variables were substituted for
+ * QUERY_EXTENDED and QUERY_PREPARED
* first_line A short, single-line extract of 'lines', for error reporting.
* type SQL_COMMAND or META_COMMAND
* meta The type of meta-command, with META_NONE/GSET/ASET if command
@@ -517,10 +531,12 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
* aset do gset on all possible queries of a combined query (\;).
* expr Parsed expression, if needed.
* stats Time spent in this command.
+ * mutex For delayed initializations of SQL commands.
*/
typedef struct Command
{
PQExpBufferData lines;
+ bool substituted;
char *first_line;
int type;
MetaCommand meta;
@@ -529,6 +545,7 @@ typedef struct Command
char *varprefix;
PgBenchExpr *expr;
SimpleStats stats;
+ pthread_mutex_t mutex;
} Command;
typedef struct ParsedScript
@@ -613,6 +630,7 @@ static void clear_socket_set(socket_set *sa);
static void add_socket_to_set(socket_set *sa, int fd, int idx);
static int wait_on_socket_set(socket_set *sa, int64 usecs);
static bool socket_has_input(socket_set *sa, int fd, int idx);
+static bool makeVariablesParameters(CState *st, Command *cmd);
/* callback functions for our flex lexer */
@@ -1273,7 +1291,7 @@ lookupVariable(CState *st, char *name)
/* Get the value of a variable, in string form; returns NULL if unknown */
static char *
-getVariable(CState *st, char *name)
+getVariable(CState *st, char *name, bool *isnull)
{
Variable *var;
char stringform[64];
@@ -1282,6 +1300,9 @@ getVariable(CState *st, char *name)
if (var == NULL)
return NULL; /* not found */
+ if (isnull != NULL)
+ *isnull = var->value.type == PGBT_NULL;
+
if (var->svalue)
return var->svalue; /* we have it in string form */
@@ -1377,6 +1398,9 @@ makeVariableValue(Variable *var)
* "src/bin/pgbench/exprscan.l". Also see parseVariable(), below.
*
* Note: this static function is copied from "src/bin/psql/variables.c"
+ * but changed to disallow variable names starting with a figure.
+ *
+ * FIXME does this change needs to be reverted?
*/
static bool
valid_variable_name(const char *name)
@@ -1387,6 +1411,15 @@ valid_variable_name(const char *name)
if (*ptr == '\0')
return false;
+ /* must not start with [0-9] */
+ if (IS_HIGHBIT_SET(*ptr) ||
+ strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+ "_", *ptr) != NULL)
+ ptr++;
+ else
+ return false;
+
+ /* remainder characters can include [0-9] */
while (*ptr)
{
if (IS_HIGHBIT_SET(*ptr) ||
@@ -1502,6 +1535,16 @@ putVariableInt(CState *st, const char *context, char *name, int64 value)
return putVariableValue(st, context, name, &val);
}
+/* Assign variable to NULL, return false on bad name */
+static bool
+putVariableNull(CState *st, const char *context, char *name)
+{
+ PgBenchValue val;
+
+ setNullValue(&val);
+ return putVariableValue(st, context, name, &val);
+}
+
/*
* Parse a possible variable reference (:varname).
*
@@ -1516,14 +1559,22 @@ parseVariable(const char *sql, int *eaten)
int i = 0;
char *name;
- do
- {
+ /* skip ':' */
+ i++;
+
+ /* first char of name must be valid but not [0-9] */
+ if (IS_HIGHBIT_SET(sql[i]) ||
+ strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+ "_", sql[i]) != NULL)
+ i++;
+ else
+ return NULL;
+
+ /* then proceed to check other chars */
+ while (IS_HIGHBIT_SET(sql[i]) ||
+ strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+ "_0123456789", sql[i]) != NULL)
i++;
- } while (IS_HIGHBIT_SET(sql[i]) ||
- strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
- "_0123456789", sql[i]) != NULL);
- if (i == 1)
- return NULL; /* no valid variable name chars */
name = pg_malloc(i);
memcpy(name, &sql[1], i - 1);
@@ -1575,7 +1626,7 @@ assignVariables(CState *st, char *sql)
continue;
}
- val = getVariable(st, name);
+ val = getVariable(st, name, NULL);
free(name);
if (val == NULL)
{
@@ -1592,10 +1643,12 @@ assignVariables(CState *st, char *sql)
static void
getQueryParams(CState *st, const Command *command, const char **params)
{
- int i;
-
- for (i = 0; i < command->argc - 1; i++)
- params[i] = getVariable(st, command->argv[i + 1]);
+ for (int i = 0; i < command->argc - 1; i++)
+ {
+ bool isnull;
+ char *sval = getVariable(st, command->argv[i + 1], &isnull);
+ params[i] = isnull ? NULL : sval;
+ }
}
static char *
@@ -2535,7 +2588,7 @@ runShellCommand(CState *st, char *variable, char **argv, int argc)
{
arg = argv[i] + 1; /* a string literal starting with colons */
}
- else if ((arg = getVariable(st, argv[i] + 1)) == NULL)
+ else if ((arg = getVariable(st, argv[i] + 1, NULL)) == NULL)
{
pg_log_error("%s: undefined variable \"%s\"", argv[0], argv[i]);
return false;
@@ -2656,13 +2709,29 @@ sendCommand(CState *st, Command *command)
}
else if (querymode == QUERY_EXTENDED)
{
- const char *sql = command->argv[0];
const char *params[MAX_ARGS];
+ /* do not try the mutex unless it will be necessary */
+ if (!command->substituted)
+ {
+ pthread_mutex_lock(&command->mutex);
+ if (!command->substituted)
+ {
+ if (!makeVariablesParameters(st, command))
+ {
+ pg_log_fatal("client %d script %s command %d pgbench variable substitution failed",
+ st->id, sql_script[st->use_file].desc, st->command);
+ exit(1);
+ }
+ command->substituted = true;
+ }
+ pthread_mutex_unlock(&command->mutex);
+ }
+
getQueryParams(st, command, params);
- pg_log_debug("client %d sending %s", st->id, sql);
- r = PQsendQueryParams(st->con, sql, command->argc - 1,
+ pg_log_debug("client %d sending %s", st->id, command->argv[0]);
+ r = PQsendQueryParams(st->con, command->argv[0], command->argc - 1,
NULL, params, NULL, NULL, 0);
}
else if (querymode == QUERY_PREPARED)
@@ -2670,31 +2739,42 @@ sendCommand(CState *st, Command *command)
char name[MAX_PREPARE_NAME];
const char *params[MAX_ARGS];
- if (!st->prepared[st->use_file])
+ /* do not try the mutex unless it will be necessary */
+ if (!command->substituted)
{
- int j;
- Command **commands = sql_script[st->use_file].commands;
-
- for (j = 0; commands[j] != NULL; j++)
+ pthread_mutex_lock(&command->mutex);
+ if (!command->substituted)
{
- PGresult *res;
- char name[MAX_PREPARE_NAME];
-
- if (commands[j]->type != SQL_COMMAND)
- continue;
- preparedStatementName(name, st->use_file, j);
- res = PQprepare(st->con, name,
- commands[j]->argv[0], commands[j]->argc - 1, NULL);
- if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pg_log_error("%s", PQerrorMessage(st->con));
- PQclear(res);
+ if (!makeVariablesParameters(st, command))
+ {
+ pg_log_fatal("client %d script %s command %d pgbench variable substitution failed",
+ st->id, sql_script[st->use_file].desc, st->command);
+ exit(1);
+ }
+ command->substituted = true;
}
- st->prepared[st->use_file] = true;
+ pthread_mutex_unlock(&command->mutex);
}
- getQueryParams(st, command, params);
preparedStatementName(name, st->use_file, st->command);
+ /* each client must prepare each sql command once */
+ if (!st->prepared[st->use_file][st->command])
+ {
+ PGresult *res;
+
+ res = PQprepare(st->con, name,
+ command->argv[0], command->argc - 1, NULL);
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pg_log_error("%s", PQerrorMessage(st->con));
+
+ PQclear(res);
+
+ st->prepared[st->use_file][st->command] = true;
+ }
+
+ getQueryParams(st, command, params);
+
pg_log_debug("client %d sending %s", st->id, name);
r = PQsendQueryPrepared(st->con, name, command->argc - 1,
params, NULL, NULL, 0);
@@ -2785,7 +2865,9 @@ readCommandResponse(CState *st, MetaCommand meta, char *varprefix)
varname = psprintf("%s%s", varprefix, varname);
/* store last row result as a string */
- if (!putVariable(st, meta == META_ASET ? "aset" : "gset", varname,
+ if (PQgetisnull(res, ntuples - 1, fld) ?
+ !putVariableNull(st, meta == META_ASET ? "aset" : "gset", varname) :
+ !putVariable(st, meta == META_ASET ? "aset" : "gset", varname,
PQgetvalue(res, ntuples - 1, fld)))
{
/* internal error */
@@ -2848,7 +2930,7 @@ evaluateSleep(CState *st, int argc, char **argv, int *usecs)
if (*argv[1] == ':')
{
- if ((var = getVariable(st, argv[1] + 1)) == NULL)
+ if ((var = getVariable(st, argv[1] + 1, NULL)) == NULL)
{
pg_log_error("%s: undefined variable \"%s\"", argv[0], argv[1] + 1);
return false;
@@ -4301,11 +4383,15 @@ GetTableInfo(PGconn *con, bool scale_given)
}
/*
- * Replace :param with $n throughout the command's SQL text, which
+ * Roughly replace :param with $n throughout the command's SQL text, which
* is a modifiable string in cmd->lines.
+ *
+ * Wherever :whatever is found, it is tried as a possible a variable name.
+ *
+ * FIXME should it do minimal lexing (in s/d quotes, ...)?
*/
static bool
-parseQuery(Command *cmd)
+makeVariablesParameters(CState *st, Command *cmd)
{
char *sql,
*p;
@@ -4317,15 +4403,23 @@ parseQuery(Command *cmd)
{
char var[13];
char *name;
+ char *val;
int eaten;
name = parseVariable(p, &eaten);
if (name == NULL)
{
while (*p == ':')
- {
p++;
- }
+ continue;
+ }
+
+ /* skip if variable is undefined? */
+ val = getVariable(st, name, NULL);
+ if (val == NULL)
+ {
+ pg_free(name);
+ p++;
continue;
}
@@ -4449,6 +4543,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
my_command = (Command *) pg_malloc(sizeof(Command));
initPQExpBuffer(&my_command->lines);
appendPQExpBufferStr(&my_command->lines, p);
+ my_command->substituted = false;
my_command->first_line = NULL; /* this is set later */
my_command->type = SQL_COMMAND;
my_command->meta = META_NONE;
@@ -4456,6 +4551,7 @@ create_sql_command(PQExpBuffer buf, const char *source)
memset(my_command->argv, 0, sizeof(my_command->argv));
my_command->varprefix = NULL; /* allocated later, if needed */
my_command->expr = NULL;
+ pthread_mutex_init(&my_command->mutex, NULL);
initSimpleStats(&my_command->stats);
return my_command;
@@ -4496,20 +4592,11 @@ postprocess_sql_command(Command *my_command)
buffer[strcspn(buffer, "\n\r")] = '\0';
my_command->first_line = pg_strdup(buffer);
- /* parse query if necessary */
- switch (querymode)
+ /* adjust argv[0] on simple query */
+ if (querymode == QUERY_SIMPLE)
{
- case QUERY_SIMPLE:
- my_command->argv[0] = my_command->lines.data;
- my_command->argc++;
- break;
- case QUERY_EXTENDED:
- case QUERY_PREPARED:
- if (!parseQuery(my_command))
- exit(1);
- break;
- default:
- exit(1);
+ my_command->argv[0] = my_command->lines.data;
+ my_command->argc++;
}
}
@@ -4893,6 +4980,15 @@ ParseScript(const char *script, const char *desc, int weight)
psql_scan_destroy(sstate);
}
+static int
+number_of_commands(ParsedScript *ps)
+{
+ int i = 0;
+ while (ps->commands[i] != NULL)
+ i++;
+ return i + 1;
+}
+
/*
* Read the entire contents of file fd, and return it in a malloc'd buffer.
*
@@ -6018,6 +6114,9 @@ main(int argc, char **argv)
{
state[i].cstack = conditional_stack_create();
initRandomState(&state[i].cs_func_rs);
+ for (int s = 0; s < num_scripts; s++)
+ state[i].prepared[s] =
+ pg_malloc0(sizeof(bool) * number_of_commands(&sql_script[s]));
}
pg_log_debug("pghost: %s pgport: %s nclients: %d %s: %d dbName: %s",
@@ -6809,4 +6908,27 @@ pthread_join(pthread_t th, void **thread_return)
return 0;
}
+static void
+pthread_mutex_init(pthread_mutex_t *pm, void *unused)
+{
+ *pm = CreateMutex(NULL, FALSE, NULL);
+}
+
+static void
+pthread_mutex_lock(pthread_mutex_t *pm)
+{
+ WaitForSingleObject(*pm, INFINITE);
+}
+
+static void
+pthread_mutex_unlock(pthread_mutex_t *pm)
+{
+ ReleaseMutex(*pm);
+}
+
+static void
+pthread_mutex_destroy(pthread_mutex_t *pm)
+{
+ CloseHandle(*pm);
+}
#endif /* WIN32 */
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 52009c3524..986ab1bfa7 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -318,7 +318,7 @@ pgbench(
],
'server parameter logging',
{
- '001_param_2' => q{select '1' as one \gset
+ '001_param_2' => q{select '1' as one, null as two \gset
SELECT 1 / (random() / 2)::int, :one::int, :two::int;
}
});
@@ -360,7 +360,7 @@ pgbench(
],
'server parameter logging',
{
- '001_param_4' => q{select '1' as one \gset
+ '001_param_4' => q{select '1' as one, null as two \gset
SELECT 1 / (random() / 2)::int, :one::int, :two::int;
}
});
@@ -467,6 +467,8 @@ pgbench(
qr{command=98.: int 5432\b}, # :random_seed
qr{command=99.: int -9223372036854775808\b}, # min int
qr{command=100.: int 9223372036854775807\b}, # max int
+ qr{command=103.: boolean true\b},
+ qr{command=105.: boolean true\b},
],
'pgbench expressions',
{
@@ -594,6 +596,13 @@ SELECT :v0, :v1, :v2, :v3;
-- minint constant parsing
\set min debug(-9223372036854775808)
\set max debug(-(:min + 1))
+-- null handling
+\set n0 null
+SELECT NULL AS n1, 1 AS one \gset
+\set n2 debug(:n0 is null and :n1 is null and :one is not null)
+-- undefined variables are not substituted
+SELECT ':no_such_variable' = ':' || 'no_such_variable' AS eq \gset
+\set ok debug(:eq)
}
});
view thread (8+ messages) latest in thread
Message-ID: <alpine.DEB.2.22.394.2007090839210.417896@pseudo>
Permalink: ../alpine.DEB.2.22.394.2007090839210.417896@pseudo/
Also on: postgresql.org/message-id/alpine.DEB.2.22.394.2007090839210.417896@pseudo
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: coelho@cri.ensmp.fr, pgsql-hackers@lists.postgresql.org, dgrowleyml@gmail.com, tgl@sss.pgh.pa.us, jaime.soler@gmail.com
Subject: Re: pgbench and timestamps (bounced)
In-Reply-To: <alpine.DEB.2.22.394.2007090839210.417896@pseudo>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox