From df74ff4e40874ac7c63f55830f53c4e5fdfd5c6c Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Sun, 4 Oct 2026 11:24:42 +0000 Subject: [PATCH v4 09/12] Refactor pgbench variables: introduce scoped variable store Upgrade the variable management module (variable.c/h) into a scoped variable store supporting nested lexical scopes (VariableScope / VariableScopeStack): - Replace the flat Variables array with a linked VariableScope stack (root and current scope pointers) so inner scopes can shadow outer variables and be pushed/popped cleanly. - Add lifecycle and scope management functions: initVariables(), destroyVariables(), copyVariables(), var_scope_push(), var_scope_pop(), var_put_value(), and var_get_value(), and export lookupCreateVariable(), is_an_int(), and valueTypeName(). - Add const qualifiers to read-only variable name and PgBenchValue parameters. - Replace the manual per-variable startup copy loop in main() (pgbench.c) with copyVariables(&state[i].variables, &state[0].variables). --- src/bin/pgbench/pgbench.c | 22 +-- src/bin/pgbench/variable.c | 281 ++++++++++++++++++++++++++++++++----- src/bin/pgbench/variable.h | 100 ++++++++----- 3 files changed, 310 insertions(+), 93 deletions(-) diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 82b83b580da..b1a8e8d4e10 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -775,29 +775,11 @@ main(int argc, char **argv) /* copy any -D switch values to all clients */ for (i = 1; i < nclients; i++) { - int j; - state[i].id = i; - for (j = 0; j < state[0].variables.nvars; j++) - { - Variable *var = &state[0].variables.vars[j]; - - if (var->value.type != PGBT_NO_VALUE) - { - if (!putVariableValue(&state[i].variables, "startup", - var->name, &var->value)) - exit(1); - } - else - { - if (!putVariable(&state[i].variables, "startup", - var->name, var->svalue)) - exit(1); - } - } + if (!copyVariables(&state[i].variables, &state[0].variables)) + exit(1); } } - /* other CState initializations */ for (i = 0; i < nclients; i++) { diff --git a/src/bin/pgbench/variable.c b/src/bin/pgbench/variable.c index 45989742f02..39e3140aae0 100644 --- a/src/bin/pgbench/variable.c +++ b/src/bin/pgbench/variable.c @@ -20,12 +20,14 @@ #include "common/logging.h" #include "variable.h" +#define VARIABLES_ALLOC_MARGIN 8 + /* * Return whether str matches "^\s*[-+]?[0-9]+$" * * This should agree with strtoint64() on what's accepted, ignoring overflows. */ -static bool +bool is_an_int(const char *str) { const char *ptr = str; @@ -108,6 +110,140 @@ strtodouble(const char *str, bool errorOK, double *dv) return true; } +/* + * Helper to ensure variables has a valid root and current scope. + */ +static inline VariableScope * +getCurrentScope(Variables *variables) +{ + if (unlikely(variables->root == NULL)) + { + variables->root = (VariableScope *) pg_malloc0(sizeof(VariableScope)); + variables->root->vars_sorted = true; + variables->current = variables->root; + } + return variables->current; +} + +/* + * Initialize a Variables container to empty with an active root scope. + */ +void +initVariables(Variables *variables) +{ + variables->root = (VariableScope *) pg_malloc0(sizeof(VariableScope)); + variables->root->vars_sorted = true; + variables->current = variables->root; +} + +/* + * Destroy all variables across all scopes, freeing memory. + */ +void +destroyVariables(Variables *variables) +{ + VariableScope *scope; + int i; + + while (var_scope_pop(variables)) + ; + + if (variables->root) + { + scope = variables->root; + for (i = 0; i < scope->nvars; i++) + { + free(scope->vars[i].name); + free(scope->vars[i].svalue); + } + if (scope->vars) + free(scope->vars); + free(scope); + variables->root = NULL; + variables->current = NULL; + } +} + +/* + * Push a new child scope onto the stack. + */ +bool +var_scope_push(Variables *variables) +{ + VariableScope *current = getCurrentScope(variables); + VariableScope *new_scope = (VariableScope *) pg_malloc0(sizeof(VariableScope)); + + new_scope->parent = current; + new_scope->vars_sorted = true; + variables->current = new_scope; + return true; +} + +/* + * Pop the current scope from the stack, freeing all variables declared in it. + * Root scope cannot be popped; returns false if at root. + */ +bool +var_scope_pop(Variables *variables) +{ + VariableScope *current; + int i; + + if (variables->root == NULL || variables->current == NULL) + return false; + + current = variables->current; + if (current == variables->root || current->parent == NULL) + return false; + + variables->current = current->parent; + + for (i = 0; i < current->nvars; i++) + { + free(current->vars[i].name); + free(current->vars[i].svalue); + } + if (current->vars) + free(current->vars); + free(current); + + return true; +} + +/* + * Deep copy all variables from the current scope of src into dest. + */ +bool +copyVariables(Variables *dest, const Variables *src) +{ + const VariableScope *src_scope; + int j; + + if (src == NULL) + return true; + + src_scope = (src->current != NULL) ? src->current : src->root; + if (src_scope == NULL) + return true; + + for (j = 0; j < src_scope->nvars; j++) + { + const Variable *var = &src_scope->vars[j]; + + if (var->value.type != PGBT_NO_VALUE) + { + if (!putVariableValue(dest, "startup", var->name, &var->value)) + return false; + } + else + { + if (!putVariable(dest, "startup", var->name, var->svalue)) + return false; + } + } + return true; +} + /* qsort comparator for Variable array */ static int compareVariableNames(const void *v1, const void *v2) @@ -116,36 +252,53 @@ compareVariableNames(const void *v1, const void *v2) ((const Variable *) v2)->name); } -/* Locate a variable by name; returns NULL if unknown */ -Variable * -lookupVariable(Variables *variables, char *name) +/* Locate a variable by name within a single scope */ +static Variable * +lookupVariableInScope(VariableScope *scope, const char *name) { Variable key; /* On some versions of Solaris, bsearch of zero items dumps core */ - if (variables->nvars <= 0) + if (scope->nvars <= 0) return NULL; /* Sort if we have to */ - if (!variables->vars_sorted) + if (!scope->vars_sorted) { - qsort(variables->vars, variables->nvars, sizeof(Variable), + qsort(scope->vars, scope->nvars, sizeof(Variable), compareVariableNames); - variables->vars_sorted = true; + scope->vars_sorted = true; } /* Now we can search */ - key.name = name; + key.name = unconstify(char *, name); return (Variable *) bsearch(&key, - variables->vars, - variables->nvars, + scope->vars, + scope->nvars, sizeof(Variable), compareVariableNames); } +/* Locate a variable by name; returns NULL if unknown */ +Variable * +lookupVariable(Variables *variables, const char *name) +{ + VariableScope *scope = getCurrentScope(variables); + + while (scope != NULL) + { + Variable *var = lookupVariableInScope(scope, name); + + if (var != NULL) + return var; + scope = scope->parent; + } + return NULL; +} + /* Get the value of a variable, in string form; returns NULL if unknown */ char * -getVariable(Variables *variables, char *name) +getVariable(Variables *variables, const char *name) { Variable *var; char stringform[64]; @@ -287,16 +440,16 @@ valid_variable_name(const char *name) * array. */ static void -enlargeVariables(Variables *variables, int needed) +enlargeVariables(VariableScope *scope, int needed) { /* total number of variables required now */ - needed += variables->nvars; + needed += scope->nvars; - if (variables->max_vars < needed) + if (scope->max_vars < needed) { - variables->max_vars = needed + VARIABLES_ALLOC_MARGIN; - variables->vars = (Variable *) - pg_realloc_array(variables->vars, Variable, variables->max_vars); + scope->max_vars = needed + VARIABLES_ALLOC_MARGIN; + scope->vars = (Variable *) + pg_realloc_array(scope->vars, Variable, scope->max_vars); } } @@ -305,10 +458,11 @@ enlargeVariables(Variables *variables, int needed) * Caller is expected to assign a value to the variable. * Returns NULL on failure (bad name). */ -static Variable * -lookupCreateVariable(Variables *variables, const char *context, char *name) +Variable * +lookupCreateVariable(Variables *variables, const char *context, const char *name) { Variable *var; + VariableScope *current = getCurrentScope(variables); var = lookupVariable(variables, name); if (var == NULL) @@ -324,17 +478,18 @@ lookupCreateVariable(Variables *variables, const char *context, char *name) } /* Create variable at the end of the array */ - enlargeVariables(variables, 1); + enlargeVariables(current, 1); - var = &(variables->vars[variables->nvars]); + var = &(current->vars[current->nvars]); var->name = pg_strdup(name); var->svalue = NULL; + var->value.type = PGBT_NO_VALUE; /* caller is expected to initialize remaining fields */ - variables->nvars++; + current->nvars++; /* we don't re-sort the array till we have to */ - variables->vars_sorted = false; + current->vars_sorted = false; } return var; @@ -343,7 +498,7 @@ lookupCreateVariable(Variables *variables, const char *context, char *name) /* Assign a string value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ bool -putVariable(Variables *variables, const char *context, char *name, +putVariable(Variables *variables, const char *context, const char *name, const char *value) { Variable *var; @@ -366,7 +521,7 @@ putVariable(Variables *variables, const char *context, char *name, /* Assign a value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ bool -putVariableValue(Variables *variables, const char *context, char *name, +putVariableValue(Variables *variables, const char *context, const char *name, const PgBenchValue *value) { Variable *var; @@ -385,7 +540,7 @@ putVariableValue(Variables *variables, const char *context, char *name, /* Assign an integer value to a variable, creating it if need be */ /* Returns false on failure (bad name) */ bool -putVariableInt(Variables *variables, const char *context, char *name, +putVariableInt(Variables *variables, const char *context, const char *name, int64 value) { PgBenchValue val; @@ -394,6 +549,64 @@ putVariableInt(Variables *variables, const char *context, char *name, return putVariableValue(variables, context, name, &val); } +/* + * Scoped put: if local_only is true, variable is bound to the current + * innermost scope frame, shadowing outer frames. + */ +bool +var_put_value(Variables *variables, const char *name, const PgBenchValue *val, bool local_only) +{ + VariableScope *current = getCurrentScope(variables); + Variable *var; + + if (local_only) + { + var = lookupVariableInScope(current, name); + if (var == NULL) + { + if (!valid_variable_name(name)) + { + pg_log_error("invalid variable name: \"%s\"", name); + return false; + } + enlargeVariables(current, 1); + var = &(current->vars[current->nvars]); + var->name = pg_strdup(name); + var->svalue = NULL; + var->value.type = PGBT_NO_VALUE; + current->nvars++; + current->vars_sorted = false; + } + free(var->svalue); + var->svalue = NULL; + var->value = *val; + return true; + } + else + { + return putVariableValue(variables, "variable", name, val); + } +} + +/* + * Scoped get: lookup variable across scopes and ensure its value is converted. + */ +bool +var_get_value(Variables *variables, const char *name, PgBenchValue *val) +{ + Variable *var = lookupVariable(variables, name); + + if (var == NULL) + return false; + + if (!makeVariableValue(var)) + return false; + + if (val != NULL) + *val = var->value; + return true; +} + /* * Parse a possible variable reference (:varname). * @@ -430,7 +643,7 @@ parseVariable(const char *sql, int *eaten) } char * -replaceVariable(char **sql, char *param, int len, char *value) +replaceVariable(char **sql, char *param, int len, const char *value) { int valueln = strlen(value); @@ -485,8 +698,8 @@ assignVariables(Variables *variables, char *sql) return sql; } -static char * -valueTypeName(PgBenchValue *pval) +const char * +valueTypeName(const PgBenchValue *pval) { if (pval->type == PGBT_NO_VALUE) return "none"; @@ -508,7 +721,7 @@ valueTypeName(PgBenchValue *pval) /* get a value as a boolean, or tell if there is a problem */ bool -coerceToBool(PgBenchValue *pval, bool *bval) +coerceToBool(const PgBenchValue *pval, bool *bval) { if (pval->type == PGBT_BOOLEAN) { @@ -528,7 +741,7 @@ coerceToBool(PgBenchValue *pval, bool *bval) * Non zero numerical values are true, zero and NULL are false. */ bool -valueTruth(PgBenchValue *pval) +valueTruth(const PgBenchValue *pval) { switch (pval->type) { @@ -549,7 +762,7 @@ valueTruth(PgBenchValue *pval) /* get a value as an int, tell if there is a problem */ bool -coerceToInt(PgBenchValue *pval, int64 *ival) +coerceToInt(const PgBenchValue *pval, int64 *ival) { if (pval->type == PGBT_INT) { @@ -577,7 +790,7 @@ coerceToInt(PgBenchValue *pval, int64 *ival) /* get a value as a double, or tell if there is a problem */ bool -coerceToDouble(PgBenchValue *pval, double *dval) +coerceToDouble(const PgBenchValue *pval, double *dval) { if (pval->type == PGBT_DOUBLE) { diff --git a/src/bin/pgbench/variable.h b/src/bin/pgbench/variable.h index 68ab17477a7..b0f83a028da 100644 --- a/src/bin/pgbench/variable.h +++ b/src/bin/pgbench/variable.h @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * variable.h - * Variable management and value types for pgbench + * Scoped variable store and value evaluation support for pgbench * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -10,11 +10,12 @@ * *------------------------------------------------------------------------- */ -#ifndef VARIABLE_H -#define VARIABLE_H + +#ifndef PGBENCH_VARIABLE_H +#define PGBENCH_VARIABLE_H /* - * Variable types used in parser. + * Variable types used in parser and expressions. */ typedef enum { @@ -38,12 +39,6 @@ typedef struct } u; } PgBenchValue; -/* - * We don't want to allocate variables one by one; for efficiency, add a - * constant margin each time it overflows. - */ -#define VARIABLES_ALLOC_MARGIN 8 - /* * Variable definitions. * @@ -54,7 +49,7 @@ typedef struct * In this case "svalue" contains the string equivalent of the value, if we've * had occasion to compute that, or NULL if we haven't. */ -typedef struct +typedef struct Variable { char *name; /* variable's name */ char *svalue; /* its value in string form, if known */ @@ -62,45 +57,72 @@ typedef struct } Variable; /* - * Data structure for client variables. + * Scoped variable stack: each scope frame can hold local variables + * that shadow outer variables, enabling loop counters, local script + * variables, and function frame isolation. */ -typedef struct +typedef struct VariableScope { - Variable *vars; /* array of variable definitions */ - int nvars; /* number of variables */ + struct VariableScope *parent; /* Enclosing scope (NULL for root scope) */ + Variable *vars; /* array of variable definitions */ + int nvars; /* number of variables in this scope */ + int max_vars; /* allocated capacity */ + bool vars_sorted; /* are variables sorted by name? */ +} VariableScope; - /* - * The maximum number of variables that we can currently store in 'vars' - * without having to reallocate more space. We must always have max_vars - * >= nvars. - */ - int max_vars; - - bool vars_sorted; /* are variables sorted by name? */ +/* + * Container for client variables with root scope and active scope stack. + */ +typedef struct Variables +{ + VariableScope *root; /* root / global scope */ + VariableScope *current; /* top of scope stack (innermost scope) */ } Variables; +typedef Variables VariableScopeStack; + +/* Value manipulation routines */ +extern const char *valueTypeName(const PgBenchValue *pval); +extern bool coerceToBool(const PgBenchValue *pval, bool *bval); +extern bool valueTruth(const PgBenchValue *pval); +extern bool coerceToInt(const PgBenchValue *pval, int64 *ival); +extern bool coerceToDouble(const PgBenchValue *pval, double *dval); +extern void setNullValue(PgBenchValue *pv); +extern void setBoolValue(PgBenchValue *pv, bool bval); +extern void setIntValue(PgBenchValue *pv, int64 ival); +extern void setDoubleValue(PgBenchValue *pv, double dval); + +/* Parsing numbers */ +extern bool is_an_int(const char *str); extern bool strtoint64(const char *str, bool errorOK, int64 *result); extern bool strtodouble(const char *str, bool errorOK, double *dv); -extern Variable *lookupVariable(Variables *variables, char *name); -extern char *getVariable(Variables *variables, char *name); +/* Variable lifecycle & scope management */ +extern void initVariables(Variables *variables); +extern void destroyVariables(Variables *variables); +extern bool copyVariables(Variables *dest, const Variables *src); +extern bool var_scope_push(Variables *variables); +extern bool var_scope_pop(Variables *variables); + +/* Variable lookup and modification */ +extern Variable *lookupVariable(Variables *variables, const char *name); +extern Variable *lookupCreateVariable(Variables *variables, const char *context, const char *name); +extern char *getVariable(Variables *variables, const char *name); extern bool makeVariableValue(Variable *var); -extern bool putVariable(Variables *variables, const char *context, char *name, +extern bool putVariable(Variables *variables, const char *context, const char *name, const char *value); -extern bool putVariableValue(Variables *variables, const char *context, - char *name, const PgBenchValue *value); -extern bool putVariableInt(Variables *variables, const char *context, char *name, +extern bool putVariableValue(Variables *variables, const char *context, const char *name, + const PgBenchValue *value); +extern bool putVariableInt(Variables *variables, const char *context, const char *name, int64 value); + +/* High-level scoped get/put */ +extern bool var_put_value(Variables *variables, const char *name, const PgBenchValue *val, bool local_only); +extern bool var_get_value(Variables *variables, const char *name, PgBenchValue *val); + +/* String substitution helpers */ extern char *parseVariable(const char *sql, int *eaten); -extern char *replaceVariable(char **sql, char *param, int len, char *value); +extern char *replaceVariable(char **sql, char *param, int len, const char *value); extern char *assignVariables(Variables *variables, char *sql); -extern bool coerceToBool(PgBenchValue *pval, bool *bval); -extern bool valueTruth(PgBenchValue *pval); -extern bool coerceToInt(PgBenchValue *pval, int64 *ival); -extern bool coerceToDouble(PgBenchValue *pval, double *dval); -extern void setNullValue(PgBenchValue *pv); -extern void setBoolValue(PgBenchValue *pv, bool bval); -extern void setIntValue(PgBenchValue *pv, int64 ival); -extern void setDoubleValue(PgBenchValue *pv, double dval); -#endif /* VARIABLE_H */ +#endif /* PGBENCH_VARIABLE_H */ -- 2.56.0.385.gd3acb90ef8-goog