Fabien COELHO <coe...@cri.ensmp.fr> writes:
>> A better answer, perhaps, would be to store double-valued variables in
>> double format to begin with, coercing to text only when and if the value
>> is interpolated into a string.
> Yep, but that was yet more changes for a limited benefit and would have
> increase the probability that the patch would have been rejected.
>> That's probably a bigger change than we want to be putting in right now,
>> though I'm a bit tempted to go try it.
> I definitely agree that the text variable solution is pretty ugly, but it
> was the minimum change solution, and I do not have much time available.
Well, I felt like doing some minor hacking, so I went and adjusted the
code to work this way. I'm pretty happy with the result, what do you
think?
regards, tom lane
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 2a9063a..a484165 100644
*** a/src/bin/pgbench/pgbench.c
--- b/src/bin/pgbench/pgbench.c
***************
*** 38,43 ****
--- 38,44 ----
#include "portability/instr_time.h"
#include <ctype.h>
+ #include <float.h>
#include <limits.h>
#include <math.h>
#include <signal.h>
*************** const char *progname;
*** 185,195 ****
volatile bool timer_exceeded = false; /* flag from signal handler */
! /* variable definitions */
typedef struct
{
! char *name; /* variable name */
! char *value; /* its value */
} Variable;
#define MAX_SCRIPTS 128 /* max number of SQL scripts allowed */
--- 186,205 ----
volatile bool timer_exceeded = false; /* flag from signal handler */
! /*
! * Variable definitions. If a variable has a string value, "value" is that
! * value, is_numeric is false, and num_value is undefined. If the value is
! * known to be numeric, is_numeric is true and num_value contains the value
! * (in any permitted numeric variant). In this case "value" contains the
! * string equivalent of the number, if we've had occasion to compute that,
! * or NULL if we haven't.
! */
typedef struct
{
! char *name; /* variable's name */
! char *value; /* its value in string form, if known */
! bool is_numeric; /* is numeric value known? */
! PgBenchValue num_value; /* variable's value in numeric form */
} Variable;
#define MAX_SCRIPTS 128 /* max number of SQL scripts allowed */
*************** typedef struct
*** 237,243 ****
bool throttling; /* whether nap is for throttling */
bool is_throttled; /* whether transaction throttling is done */
Variable *variables; /* array of variable definitions */
! int nvariables;
int64 txn_scheduled; /* scheduled start time of transaction (usec) */
instr_time txn_begin; /* used for measuring schedule lag times */
instr_time stmt_begin; /* used for measuring statement latencies */
--- 247,254 ----
bool throttling; /* whether nap is for throttling */
bool is_throttled; /* whether transaction throttling is done */
Variable *variables; /* array of variable definitions */
! int nvariables; /* number of variables */
! bool vars_sorted; /* are variables sorted by name? */
int64 txn_scheduled; /* scheduled start time of transaction (usec) */
instr_time txn_begin; /* used for measuring schedule lag times */
instr_time stmt_begin; /* used for measuring statement latencies */
*************** static const BuiltinScript builtin_scrip
*** 363,368 ****
--- 374,381 ----
/* Function prototypes */
+ static void setIntValue(PgBenchValue *pv, int64 ival);
+ static void setDoubleValue(PgBenchValue *pv, double dval);
static bool evaluateExpr(TState *, CState *, PgBenchExpr *, PgBenchValue *);
static void doLog(TState *thread, CState *st, instr_time *now,
StatsData *agg, bool skipped, double latency, double lag);
*************** discard_response(CState *state)
*** 836,868 ****
} while (res);
}
static int
! compareVariables(const void *v1, const void *v2)
{
return strcmp(((const Variable *) v1)->name,
((const Variable *) v2)->name);
}
! static char *
! getVariable(CState *st, char *name)
{
! Variable key,
! *var;
/* On some versions of Solaris, bsearch of zero items dumps core */
if (st->nvariables <= 0)
return NULL;
key.name = name;
! var = (Variable *) bsearch((void *) &key,
! (void *) st->variables,
! st->nvariables,
! sizeof(Variable),
! compareVariables);
! if (var != NULL)
! return var->value;
else
! return NULL;
}
/* check whether the name consists of alphabets, numerals and underscores. */
--- 849,945 ----
} while (res);
}
+ /* qsort comparator for Variable array */
static int
! compareVariableNames(const void *v1, const void *v2)
{
return strcmp(((const Variable *) v1)->name,
((const Variable *) v2)->name);
}
! /* Locate a variable by name; returns NULL if unknown */
! static Variable *
! lookupVariable(CState *st, char *name)
{
! Variable key;
/* On some versions of Solaris, bsearch of zero items dumps core */
if (st->nvariables <= 0)
return NULL;
+ /* Sort if we have to */
+ if (!st->vars_sorted)
+ {
+ qsort((void *) st->variables, st->nvariables, sizeof(Variable),
+ compareVariableNames);
+ st->vars_sorted = true;
+ }
+
+ /* Now we can search */
key.name = name;
! return (Variable *) bsearch((void *) &key,
! (void *) st->variables,
! st->nvariables,
! sizeof(Variable),
! compareVariableNames);
! }
!
! /* Get the value of a variable, in string form; returns NULL if unknown */
! static char *
! getVariable(CState *st, char *name)
! {
! Variable *var;
! char stringform[64];
!
! var = lookupVariable(st, name);
! if (var == NULL)
! return NULL; /* not found */
!
! if (var->value)
! return var->value; /* we have it in string form */
!
! /* We need to produce a string equivalent of the numeric value */
! Assert(var->is_numeric);
! if (var->num_value.type == PGBT_INT)
! snprintf(stringform, sizeof(stringform),
! INT64_FORMAT, var->num_value.u.ival);
else
! {
! Assert(var->num_value.type == PGBT_DOUBLE);
! snprintf(stringform, sizeof(stringform),
! "%.*g", DBL_DIG, var->num_value.u.dval);
! }
! var->value = pg_strdup(stringform);
! return var->value;
! }
!
! /* Try to convert variable to numeric form; return false on failure */
! static bool
! makeVariableNumeric(Variable *var)
! {
! if (var->is_numeric)
! return true; /* no work */
!
! if (is_an_int(var->value))
! {
! setIntValue(&var->num_value, strtoint64(var->value));
! var->is_numeric = true;
! }
! else /* type should be double */
! {
! double dv;
!
! if (sscanf(var->value, "%lf", &dv) != 1)
! {
! fprintf(stderr,
! "malformed variable \"%s\" value: \"%s\"\n",
! var->name, var->value);
! return false;
! }
! setDoubleValue(&var->num_value, dv);
! var->is_numeric = true;
! }
! return true;
}
/* check whether the name consists of alphabets, numerals and underscores. */
*************** isLegalVariableName(const char *name)
*** 877,902 ****
return false;
}
! return true;
}
! static int
! putVariable(CState *st, const char *context, char *name, char *value)
{
! Variable key,
! *var;
!
! key.name = name;
! /* On some versions of Solaris, bsearch of zero items dumps core */
! if (st->nvariables > 0)
! var = (Variable *) bsearch((void *) &key,
! (void *) st->variables,
! st->nvariables,
! sizeof(Variable),
! compareVariables);
! else
! var = NULL;
if (var == NULL)
{
Variable *newvars;
--- 954,973 ----
return false;
}
! return (i > 0); /* must be non-empty */
}
! /*
! * Lookup a variable by name, creating it if need be.
! * Caller is expected to assign a value to the variable.
! * Returns NULL on failure (bad name).
! */
! static Variable *
! lookupCreateVariable(CState *st, const char *context, char *name)
{
! Variable *var;
+ var = lookupVariable(st, name);
if (var == NULL)
{
Variable *newvars;
*************** putVariable(CState *st, const char *cont
*** 909,917 ****
{
fprintf(stderr, "%s: invalid variable name: \"%s\"\n",
context, name);
! return false;
}
if (st->variables)
newvars = (Variable *) pg_realloc(st->variables,
(st->nvariables + 1) * sizeof(Variable));
--- 980,989 ----
{
fprintf(stderr, "%s: invalid variable name: \"%s\"\n",
context, name);
! return NULL;
}
+ /* Create variable at the end of the array */
if (st->variables)
newvars = (Variable *) pg_realloc(st->variables,
(st->nvariables + 1) * sizeof(Variable));
*************** putVariable(CState *st, const char *cont
*** 923,949 ****
var = &newvars[st->nvariables];
var->name = pg_strdup(name);
! var->value = pg_strdup(value);
st->nvariables++;
!
! qsort((void *) st->variables, st->nvariables, sizeof(Variable),
! compareVariables);
}
- else
- {
- char *val;
! /* dup then free, in case value is pointing at this variable */
! val = pg_strdup(value);
free(var->value);
! var->value = val;
! }
return true;
}
static char *
parseVariable(const char *sql, int *eaten)
{
--- 995,1066 ----
var = &newvars[st->nvariables];
var->name = pg_strdup(name);
! var->value = NULL;
! /* caller is expected to initialize remaining fields */
st->nvariables++;
! /* we don't re-sort the array till we have to */
! st->vars_sorted = false;
}
! return var;
! }
+ /* Assign a string value to a variable, creating it if need be */
+ /* Returns false on failure (bad name) */
+ static bool
+ putVariable(CState *st, const char *context, char *name, const char *value)
+ {
+ Variable *var;
+ char *val;
+
+ var = lookupCreateVariable(st, context, name);
+ if (!var)
+ return false;
+
+ /* dup then free, in case value is pointing at this variable */
+ val = pg_strdup(value);
+
+ if (var->value)
free(var->value);
! var->value = val;
! var->is_numeric = false;
!
! return true;
! }
!
! /* Assign a numeric value to a variable, creating it if need be */
! /* Returns false on failure (bad name) */
! static bool
! putVariableNumber(CState *st, const char *context, char *name,
! const PgBenchValue *value)
! {
! Variable *var;
!
! var = lookupCreateVariable(st, context, name);
! if (!var)
! return false;
!
! if (var->value)
! free(var->value);
! var->value = NULL;
! var->is_numeric = true;
! var->num_value = *value;
return true;
}
+ /* Assign an integer value to a variable, creating it if need be */
+ /* Returns false on failure (bad name) */
+ static bool
+ putVariableInt(CState *st, const char *context, char *name, int64 value)
+ {
+ PgBenchValue val;
+
+ setIntValue(&val, value);
+ return putVariableNumber(st, context, name, &val);
+ }
+
static char *
parseVariable(const char *sql, int *eaten)
{
*************** evalFunc(TState *thread, CState *st,
*** 1260,1266 ****
else
{
Assert(varg->type == PGBT_DOUBLE);
! fprintf(stderr, "double %f\n", varg->u.dval);
}
*retval = *varg;
--- 1377,1383 ----
else
{
Assert(varg->type == PGBT_DOUBLE);
! fprintf(stderr, "double %.*g\n", DBL_DIG, varg->u.dval);
}
*retval = *varg;
*************** evaluateExpr(TState *thread, CState *st,
*** 1454,1485 ****
case ENODE_VARIABLE:
{
! char *var;
! if ((var = getVariable(st, expr->u.variable.varname)) == NULL)
{
fprintf(stderr, "undefined variable \"%s\"\n",
expr->u.variable.varname);
return false;
}
! if (is_an_int(var))
! {
! setIntValue(retval, strtoint64(var));
! }
! else /* type should be double */
! {
! double dv;
! if (sscanf(var, "%lf", &dv) != 1)
! {
! fprintf(stderr,
! "malformed variable \"%s\" value: \"%s\"\n",
! expr->u.variable.varname, var);
! return false;
! }
! setDoubleValue(retval, dv);
! }
return true;
}
--- 1571,1589 ----
case ENODE_VARIABLE:
{
! Variable *var;
! if ((var = lookupVariable(st, expr->u.variable.varname)) == NULL)
{
fprintf(stderr, "undefined variable \"%s\"\n",
expr->u.variable.varname);
return false;
}
! if (!makeVariableNumeric(var))
! return false;
+ *retval = var->num_value;
return true;
}
*************** runShellCommand(CState *st, char *variab
*** 1596,1603 ****
argv[0], res);
return false;
}
! snprintf(res, sizeof(res), "%d", retval);
! if (!putVariable(st, "setshell", variable, res))
return false;
#ifdef DEBUG
--- 1700,1706 ----
argv[0], res);
return false;
}
! if (!putVariableInt(st, "setshell", variable, retval))
return false;
#ifdef DEBUG
*************** top:
*** 1973,1979 ****
if (pg_strcasecmp(argv[0], "set") == 0)
{
- char res[64];
PgBenchExpr *expr = commands[st->state]->expr;
PgBenchValue result;
--- 2076,2081 ----
*************** top:
*** 1983,1997 ****
return true;
}
! if (result.type == PGBT_INT)
! sprintf(res, INT64_FORMAT, result.u.ival);
! else
! {
! Assert(result.type == PGBT_DOUBLE);
! sprintf(res, "%.18e", result.u.dval);
! }
!
! if (!putVariable(st, argv[0], argv[1], res))
{
st->ecnt++;
return true;
--- 2085,2091 ----
return true;
}
! if (!putVariableNumber(st, argv[0], argv[1], &result))
{
st->ecnt++;
return true;
*************** main(int argc, char **argv)
*** 3325,3332 ****
PGresult *res;
char *env;
- char val[64];
-
progname = get_progname(argv[0]);
if (argc > 1)
--- 3419,3424 ----
*************** main(int argc, char **argv)
*** 3767,3774 ****
state[i].id = i;
for (j = 0; j < state[0].nvariables; j++)
{
! if (!putVariable(&state[i], "startup", state[0].variables[j].name, state[0].variables[j].value))
! exit(1);
}
}
}
--- 3859,3878 ----
state[i].id = i;
for (j = 0; j < state[0].nvariables; j++)
{
! Variable *var = &state[0].variables[j];
!
! if (var->is_numeric)
! {
! if (!putVariableNumber(&state[i], "startup",
! var->name, &var->num_value))
! exit(1);
! }
! else
! {
! if (!putVariable(&state[i], "startup",
! var->name, var->value))
! exit(1);
! }
}
}
}
*************** main(int argc, char **argv)
*** 3834,3845 ****
* :scale variables normally get -s or database scale, but don't override
* an explicit -D switch
*/
! if (getVariable(&state[0], "scale") == NULL)
{
- snprintf(val, sizeof(val), "%d", scale);
for (i = 0; i < nclients; i++)
{
! if (!putVariable(&state[i], "startup", "scale", val))
exit(1);
}
}
--- 3938,3948 ----
* :scale variables normally get -s or database scale, but don't override
* an explicit -D switch
*/
! if (lookupVariable(&state[0], "scale") == NULL)
{
for (i = 0; i < nclients; i++)
{
! if (!putVariableInt(&state[i], "startup", "scale", scale))
exit(1);
}
}
*************** main(int argc, char **argv)
*** 3848,3859 ****
* Define a :client_id variable that is unique per connection. But don't
* override an explicit -D switch.
*/
! if (getVariable(&state[0], "client_id") == NULL)
{
for (i = 0; i < nclients; i++)
{
! snprintf(val, sizeof(val), "%d", i);
! if (!putVariable(&state[i], "startup", "client_id", val))
exit(1);
}
}
--- 3951,3961 ----
* Define a :client_id variable that is unique per connection. But don't
* override an explicit -D switch.
*/
! if (lookupVariable(&state[0], "client_id") == NULL)
{
for (i = 0; i < nclients; i++)
{
! if (!putVariableInt(&state[i], "startup", "client_id", i))
exit(1);
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
