Re: pgbench and timestamps (bounced)
[Resent on hackers for CF registration, sorry for the noise]
Hello Tom,
The attached patch fixes some of the underlying problems reported by delaying
the :var to $1 substitution to the last possible moments, so that what
variables are actually defined is known. PREPARE-ing is also delayed to after
these substitutions are done.
It requires a mutex around the commands, I tried to do some windows
implementation which may or may not work.
The attached patch fixes (2) & (3) for extended & prepared.
I have a doubt about fixing (1) because it would be a significant behavioral
change and it requires changing the replace routine significantly to check for
quoting, comments, and so on. This means that currently ':var' is still broken
under -M extended & prepared, I could only break it differently by providing a
nicer error message and also break it under simple whereas it currently works
there. I'm not thrilled by spending efforts to do that.
The patches change the name of "parseQuery" to "makeVariablesParameters",
because it was not actually parsing any query. Maybe the new name could be
improved.
In passing, there was a bug in how NULL was passed, which I tried to fix
as well.
I don't often do much with pgbench and variables, but there are a few
things that surprise me here.
1) That pgbench replaces variables within single quotes, and;
2) that we still think it's a variable name when it starts with a digit,
and;
3) We replace variables that are undefined.Also (4) this only happens when in non-simple query mode --- the
example works fine without "-M prepared".After looking around in the code, it seems like the core of the issue
is that pgbench.c's parseQuery() doesn't check whether a possible
variable name is actually defined, unlike assignVariables() which is
what does the same job in simple query mode. So that explains the
behavioral difference.
Yes.
The reason for doing that probably was that parseQuery() is run when
the input file is read, so that relevant variables might not be set
yet. We could fix that by postponing the work to be done at first
execution of the query, as is already the case for PQprepare'ing the
query.
Yep, done at first execution of the Command, so that variables are known.
Also, after further thought I realize that (1) absolutely is a bug
in the non-simple query modes, whatever you think about it in simple
mode. The non-simple modes are trying to pass the variable values
as extended-query-protocol parameters, and the backend is not going
to recognize $n inside a literal as being a parameter.
Yep. See my comments above.
If we fixed (1) and (3) I think there wouldn't be any great need
to tighten up (2).
I did (2) but not (1), for now.
--
Fabien.
Attachments:
pgbench-var-fix-1.patchtext/x-diff; name=pgbench-var-fix-1.patchDownload
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 8e728dc094..7436210fd4 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..09ccf05db5 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, const 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)
}
});
Attached v2 fixes some errors, per cfbot.
--
Fabien.
Attachments:
pgbench-var-fix-2.patchtext/x-diff; NAME=pgbench-var-fix-2.patchDownload
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)
}
});
Fabien COELHO <coelho@cri.ensmp.fr> writes:
[Resent on hackers for CF registration, sorry for the noise]
For the record, the original thread is at
/messages/by-id/CAKVUGgQaZVAUi1Ex41H4wrru=FU+MfwgjG0aM1br6st7sz31Vw@mail.gmail.com
(I tried but failed to attach that thread to the CF entry, so we'll
have to settle for leaving a breadcrumb in this thread.)
It requires a mutex around the commands, I tried to do some windows
implementation which may or may not work.
Ugh, I'd really rather not do that. Even disregarding the effects
of a mutex, though, my initial idea for fixing this has a big problem:
if we postpone PREPARE of the query until first execution, then it's
happening during timed execution of the benchmark scenario and thus
distorting the timing figures. (Maybe if we'd always done it like
that, it'd be okay, but I'm quite against changing the behavior now
that it's stood for a long time.)
However, perhaps there's more than one way to fix this. Once we've
scanned all of the script and seen all the \set commands, we know
(in principle) the set of all variable names that are in use.
So maybe we could fix this by
(1) During the initial scan of the script, make variable-table
entries for every \set argument, with the values shown as undefined
for the moment. Do not try to parse SQL commands in this scan,
just collect them.
(2) Make another scan in which we identify variable references
in the SQL commands and issue PREPAREs (if enabled).
(3) Perform the timed run.
This avoids any impact of this bug fix on the semantics or timing
of the benchmark proper. I'm not sure offhand whether this
approach makes any difference for the concerns you had about
identifying/suppressing variable references inside quotes.
regards, tom lane
Hello Tom,
It requires a mutex around the commands, I tried to do some windows
implementation which may or may not work.Ugh, I'd really rather not do that. Even disregarding the effects
of a mutex, though, my initial idea for fixing this has a big problem:
if we postpone PREPARE of the query until first execution, then it's
happening during timed execution of the benchmark scenario and thus
distorting the timing figures. (Maybe if we'd always done it like
that, it'd be okay, but I'm quite against changing the behavior now
that it's stood for a long time.)
Hmmm.
Prepare is done *once* per client, ISTM that the impact on any
statistically significant benchmark is nul in practice, or it would mean
that the benchmark settings are too low.
Second, the mutex is only used when absolutely necessary, only for the
substitution part of the query (replacing :stuff by ?), because scripts
are shared between threads. This is just once, in an unlikely case
occuring at the beginning.
However, perhaps there's more than one way to fix this. Once we've
scanned all of the script and seen all the \set commands, we know
(in principle) the set of all variable names that are in use.
So maybe we could fix this by(1) During the initial scan of the script, make variable-table
entries for every \set argument, with the values shown as undefined
for the moment. Do not try to parse SQL commands in this scan,
just collect them.
The issue with this approach is
SELECT 1 AS one \gset pref_
which will generate a "pref_one" variable, and these names cannot be
guessed without SQL parsing and possibly execution. That is why the
preparation is delayed to when the variables are actually known.
(2) Make another scan in which we identify variable references
in the SQL commands and issue PREPAREs (if enabled).
(3) Perform the timed run.
This avoids any impact of this bug fix on the semantics or timing
of the benchmark proper. I'm not sure offhand whether this
approach makes any difference for the concerns you had about
identifying/suppressing variable references inside quotes.
I do not think this plan is workable, because of the \gset issue.
I do not see that the conditional mutex and delayed PREPARE would have any
significant (measurable) impact on an actual (reasonable) benchmark run.
A workable solution would be that each client actually execute each script
once before starting the actual benchmark. It would still need a mutex and
also a sync barrier (which I'm proposing in some other thread). However
this may raise some other issues because then some operations would be
trigger out of the benchmarking run, which may or may not be desirable.
So I'm not to keen to go that way, and I think the proposed solution is
reasonable from a benchmarking point of view as the impact is minimal,
although not zero.
--
Fabien.
On 11.09.2020 16:59, Fabien COELHO wrote:
Hello Tom,
It requires a mutex around the commands, I tried to do some windows
implementation which may or may not work.Ugh, I'd really rather not do that. Even disregarding the effects
of a mutex, though, my initial idea for fixing this has a big problem:
if we postpone PREPARE of the query until first execution, then it's
happening during timed execution of the benchmark scenario and thus
distorting the timing figures. (Maybe if we'd always done it like
that, it'd be okay, but I'm quite against changing the behavior now
that it's stood for a long time.)Hmmm.
Prepare is done *once* per client, ISTM that the impact on any
statistically significant benchmark is nul in practice, or it would
mean that the benchmark settings are too low.Second, the mutex is only used when absolutely necessary, only for the
substitution part of the query (replacing :stuff by ?), because
scripts are shared between threads. This is just once, in an unlikely
case occuring at the beginning.However, perhaps there's more than one way to fix this. Once we've
scanned all of the script and seen all the \set commands, we know
(in principle) the set of all variable names that are in use.
So maybe we could fix this by(1) During the initial scan of the script, make variable-table
entries for every \set argument, with the values shown as undefined
for the moment. Do not try to parse SQL commands in this scan,
just collect them.The issue with this approach is
SELECT 1 AS one \gset pref_
which will generate a "pref_one" variable, and these names cannot be
guessed without SQL parsing and possibly execution. That is why the
preparation is delayed to when the variables are actually known.(2) Make another scan in which we identify variable references
in the SQL commands and issue PREPAREs (if enabled).(3) Perform the timed run.
This avoids any impact of this bug fix on the semantics or timing
of the benchmark proper. I'm not sure offhand whether this
approach makes any difference for the concerns you had about
identifying/suppressing variable references inside quotes.I do not think this plan is workable, because of the \gset issue.
I do not see that the conditional mutex and delayed PREPARE would have
any significant (measurable) impact on an actual (reasonable)
benchmark run.A workable solution would be that each client actually execute each
script once before starting the actual benchmark. It would still need
a mutex and also a sync barrier (which I'm proposing in some other
thread). However this may raise some other issues because then some
operations would be trigger out of the benchmarking run, which may or
may not be desirable.So I'm not to keen to go that way, and I think the proposed solution
is reasonable from a benchmarking point of view as the impact is
minimal, although not zero.
CFM reminder.
Hi, this entry is "Waiting on Author" and the thread was inactive for a
while. I see this discussion still has some open questions. Are you
going to continue working on it, or should I mark it as "returned with
feedback" until a better time?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
CFM reminder.
Hi, this entry is "Waiting on Author" and the thread was inactive for a
while. I see this discussion still has some open questions. Are you
going to continue working on it, or should I mark it as "returned with
feedback" until a better time?
IMHO the proposed fix is reasonable and addresses the issue.
I have responded to Tom's remarks about it, and it is waiting for his
answer to my counter arguments. So ISTM that the patch is currently still
waiting for some feedback.
--
Fabien.
Fabien COELHO <coelho@cri.ensmp.fr> writes:
Hi, this entry is "Waiting on Author" and the thread was inactive for a
while. I see this discussion still has some open questions. Are you
going to continue working on it, or should I mark it as "returned with
feedback" until a better time?
IMHO the proposed fix is reasonable and addresses the issue.
I have responded to Tom's remarks about it, and it is waiting for his
answer to my counter arguments. So ISTM that the patch is currently still
waiting for some feedback.
It looks like my unhappiness with injecting a pthread dependency into
pgbench is going to be overtaken by events in the "option delaying
queries" thread [1]/messages/by-id/20200227180100.zyvjwzcpiokfsqm2@alap3.anarazel.de. However, by the same token there are some conflicts
between the two patchsets, and also I prefer the other thread's approach
to portability (i.e. do it honestly, not with a private portability layer
in pgbench.c). So I'm inclined to put the parts of this patch that
require pthreads on hold till that lands.
What remains that we could do now, and perhaps back-patch, is point (2)
i.e. disallow digits as the first character of a pgbench variable name.
That would be enough to "solve" the original bug report, and it does seem
like it could be back-patched, while we're certainly not going to risk
back-patching anything as portability-fraught as introducing mutexes.
regards, tom lane
[1]: /messages/by-id/20200227180100.zyvjwzcpiokfsqm2@alap3.anarazel.de
Hello Tom,
Hi, this entry is "Waiting on Author" and the thread was inactive for a
while. I see this discussion still has some open questions. Are you
going to continue working on it, or should I mark it as "returned with
feedback" until a better time?IMHO the proposed fix is reasonable and addresses the issue.
I have responded to Tom's remarks about it, and it is waiting for his
answer to my counter arguments. So ISTM that the patch is currently still
waiting for some feedback.It looks like my unhappiness with injecting a pthread dependency into
pgbench is going to be overtaken by events in the "option delaying
queries" thread [1]. However, by the same token there are some conflicts
between the two patchsets, and also I prefer the other thread's approach
to portability (i.e. do it honestly, not with a private portability layer
in pgbench.c). So I'm inclined to put the parts of this patch that
require pthreads on hold till that lands.
Ok. This is fair enough. Portability is a pain thanks to Windows vs MacOS
vs Linux approaches of implementing or not a standard.
What remains that we could do now, and perhaps back-patch, is point (2)
i.e. disallow digits as the first character of a pgbench variable name.
I'm fine with that.
That would be enough to "solve" the original bug report, and it does seem
like it could be back-patched, while we're certainly not going to risk
back-patching anything as portability-fraught as introducing mutexes.
Sure.
I'm unable to do much pg work at the moment, but this should be eased
quite soon.
[1] /messages/by-id/20200227180100.zyvjwzcpiokfsqm2@alap3.anarazel.de
--
Fabien Coelho - CRI, MINES ParisTech
Fabien COELHO <coelho@cri.ensmp.fr> writes:
What remains that we could do now, and perhaps back-patch, is point (2)
i.e. disallow digits as the first character of a pgbench variable name.
I'm fine with that.
That would be enough to "solve" the original bug report, and it does seem
like it could be back-patched, while we're certainly not going to risk
back-patching anything as portability-fraught as introducing mutexes.
Sure.
OK. I've pushed a patch that just does that much, and marked the
commitfest entry closed. After the other thing lands, please rebase
and resubmit what remains of this patch.
regards, tom lane