diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 9915731..042d277 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -3213,9 +3213,8 @@ bar list. If set to a value of ignoredups, lines matching the previous history line are not entered. A value of ignoreboth combines the two options. If - unset, or if set to none (or any other value - than those above), all lines read in interactive mode are - saved on the history list. + unset, or if set to none, all lines read + in interactive mode are saved on the history list. diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 4139b77..091a138 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -253,26 +253,31 @@ exec_command(const char *cmd, opt1 = read_connect_arg(scan_state); if (opt1 != NULL && strncmp(opt1, prefix, sizeof(prefix) - 1) == 0) { - reuse_previous = - ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix) ? - TRI_YES : TRI_NO; - - free(opt1); - opt1 = read_connect_arg(scan_state); + bool on_off; + success = ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix, &on_off); + if (success) + { + reuse_previous = on_off ? TRI_YES : TRI_NO; + free(opt1); + opt1 = read_connect_arg(scan_state); + } } else reuse_previous = TRI_DEFAULT; - opt2 = read_connect_arg(scan_state); - opt3 = read_connect_arg(scan_state); - opt4 = read_connect_arg(scan_state); + if (success) /* do not attempt to connect if reuse_previous argument was invalid */ + { + opt2 = read_connect_arg(scan_state); + opt3 = read_connect_arg(scan_state); + opt4 = read_connect_arg(scan_state); - success = do_connect(reuse_previous, opt1, opt2, opt3, opt4); + success = do_connect(reuse_previous, opt1, opt2, opt3, opt4); + free(opt2); + free(opt3); + free(opt4); + } free(opt1); - free(opt2); - free(opt3); - free(opt4); } /* \cd */ @@ -1548,7 +1553,7 @@ exec_command(const char *cmd, OT_NORMAL, NULL, false); if (opt) - pset.timing = ParseVariableBool(opt, "\\timing"); + success = ParseVariableBool(opt, "\\timing", &pset.timing); else pset.timing = !pset.timing; if (!pset.quiet) @@ -2660,7 +2665,16 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) if (value && pg_strcasecmp(value, "auto") == 0) popt->topt.expanded = 2; else if (value) - popt->topt.expanded = ParseVariableBool(value, param); + { + bool on_off; + if (ParseVariableBool(value, NULL, &on_off)) + popt->topt.expanded = on_off ? 1 : 0; + else + { + PsqlVarEnumError(param, value, "on, off, auto"); + return false; + } + } else popt->topt.expanded = !popt->topt.expanded; } @@ -2669,7 +2683,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "numericlocale") == 0) { if (value) - popt->topt.numericLocale = ParseVariableBool(value, param); + return ParseVariableBool(value, param, &popt->topt.numericLocale); else popt->topt.numericLocale = !popt->topt.numericLocale; } @@ -2724,7 +2738,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0) { if (value) - popt->topt.tuples_only = ParseVariableBool(value, param); + return ParseVariableBool(value, param, &popt->topt.tuples_only); else popt->topt.tuples_only = !popt->topt.tuples_only; } @@ -2756,10 +2770,13 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) popt->topt.pager = 2; else if (value) { - if (ParseVariableBool(value, param)) - popt->topt.pager = 1; - else - popt->topt.pager = 0; + bool on_off; + if (!ParseVariableBool(value, NULL, &on_off)) + { + PsqlVarEnumError(param, value, "on, off, always"); + return false; + } + popt->topt.pager = on_off ? 1 : 0; } else if (popt->topt.pager == 1) popt->topt.pager = 0; @@ -2778,7 +2795,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) else if (strcmp(param, "footer") == 0) { if (value) - popt->topt.default_footer = ParseVariableBool(value, param); + return ParseVariableBool(value, param, &popt->topt.default_footer); else popt->topt.default_footer = !popt->topt.default_footer; } diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c index 972bea4..3e3e97a 100644 --- a/src/bin/psql/input.c +++ b/src/bin/psql/input.c @@ -541,7 +541,7 @@ finishInput(void) { int hist_size; - hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1, true); + hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1); (void) saveHistory(psql_history, hist_size); free(psql_history); psql_history = NULL; diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c index bb306a4..dc25b4b 100644 --- a/src/bin/psql/mainloop.c +++ b/src/bin/psql/mainloop.c @@ -162,7 +162,7 @@ MainLoop(FILE *source) /* This tries to mimic bash's IGNOREEOF feature. */ count_eof++; - if (count_eof < GetVariableNum(pset.vars, "IGNOREEOF", 0, 10, false)) + if (count_eof < GetVariableNum(pset.vars, "IGNOREEOF", 0, 10)) { if (!pset.quiet) printf(_("Use \"\\q\" to leave %s.\n"), pset.progname); diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 85aac4a..0b4f43e 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -786,43 +786,71 @@ showVersion(void) * This isn't an amazingly good place for them, but neither is anywhere else. */ -static void + +static bool autocommit_hook(const char *newval) { - pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT"); + bool autocommit; + if (ParseVariableBool(newval, "AUTOCOMMIT", &autocommit)) + { + /* + * Switching AUTOCOMMIT from OFF to ON is not allowed when inside a + * transaction, because it cannot be effective until the current + * transaction is ended. + */ + if (autocommit && !pset.autocommit && + pset.db && PQtransactionStatus(pset.db) == PQTRANS_INTRANS) + { + psql_error("cannot set AUTOCOMMIT to %s when inside a transaction\n", newval); + } + else + { + pset.autocommit = autocommit; + return true; + } + } + return false; } -static void +static bool on_error_stop_hook(const char *newval) { - pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP"); + return ParseVariableBool(newval, "ON_ERROR_STOP", &pset.on_error_stop); } -static void +static bool quiet_hook(const char *newval) { - pset.quiet = ParseVariableBool(newval, "QUIET"); + return ParseVariableBool(newval, "QUIET", &pset.quiet); } -static void +static bool singleline_hook(const char *newval) { - pset.singleline = ParseVariableBool(newval, "SINGLELINE"); + return ParseVariableBool(newval, "SINGLELINE", &pset.singleline); } -static void +static bool singlestep_hook(const char *newval) { - pset.singlestep = ParseVariableBool(newval, "SINGLESTEP"); + return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep); } -static void +static bool fetch_count_hook(const char *newval) { - pset.fetch_count = ParseVariableNum(newval, -1, -1, false); + if (newval == NULL) + pset.fetch_count = -1; /* default value */ + else if (!ParseVariableNum(newval, &pset.fetch_count)) + { + psql_error("Invalid value \"%s\" for \"%s\"\nAn integer is expected\n", + newval, "FETCH_COUNT"); + return false; + } + return true; } -static void +static bool echo_hook(const char *newval) { if (newval == NULL) @@ -837,39 +865,55 @@ echo_hook(const char *newval) pset.echo = PSQL_ECHO_NONE; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "ECHO", "none"); - pset.echo = PSQL_ECHO_NONE; + PsqlVarEnumError("ECHO", newval, "none, errors, queries, all"); + return false; } + return true; } -static void +static bool echo_hidden_hook(const char *newval) { if (newval == NULL) pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; else if (pg_strcasecmp(newval, "noexec") == 0) pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC; - else if (ParseVariableBool(newval, "ECHO_HIDDEN")) - pset.echo_hidden = PSQL_ECHO_HIDDEN_ON; - else /* ParseVariableBool printed msg if needed */ - pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; + else + { + bool on_off; + if (ParseVariableBool(newval, NULL, &on_off)) + pset.echo_hidden = on_off ? PSQL_ECHO_HIDDEN_ON : PSQL_ECHO_HIDDEN_OFF; + else + { + PsqlVarEnumError("ECHO_HIDDEN", newval, "on, off, noexec"); + return false; + } + } + return true; } -static void +static bool on_error_rollback_hook(const char *newval) { if (newval == NULL) pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF; else if (pg_strcasecmp(newval, "interactive") == 0) pset.on_error_rollback = PSQL_ERROR_ROLLBACK_INTERACTIVE; - else if (ParseVariableBool(newval, "ON_ERROR_ROLLBACK")) - pset.on_error_rollback = PSQL_ERROR_ROLLBACK_ON; - else /* ParseVariableBool printed msg if needed */ - pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF; + else + { + bool on_off; + if (ParseVariableBool(newval, NULL, &on_off)) + pset.on_error_rollback = on_off ? PSQL_ERROR_ROLLBACK_ON : PSQL_ERROR_ROLLBACK_OFF; + else + { + PsqlVarEnumError("ON_ERROR_ROLLBACK", newval, "on, off, interactive"); + return false; + } + } + return true; } -static void +static bool comp_keyword_case_hook(const char *newval) { if (newval == NULL) @@ -884,13 +928,14 @@ comp_keyword_case_hook(const char *newval) pset.comp_case = PSQL_COMP_CASE_LOWER; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "COMP_KEYWORD_CASE", "preserve-upper"); - pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER; + PsqlVarEnumError("COMP_KEYWORD_CASE", newval, + "lower, upper, preserve-lower, preserve-upper"); + return false; } + return true; } -static void +static bool histcontrol_hook(const char *newval) { if (newval == NULL) @@ -905,31 +950,35 @@ histcontrol_hook(const char *newval) pset.histcontrol = hctl_none; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "HISTCONTROL", "none"); - pset.histcontrol = hctl_none; + PsqlVarEnumError("HISTCONTROL", newval, + "none, ignorespace, ignoredups, ignoreboth"); + return false; } + return true; } -static void +static bool prompt1_hook(const char *newval) { pset.prompt1 = newval ? newval : ""; + return true; } -static void +static bool prompt2_hook(const char *newval) { pset.prompt2 = newval ? newval : ""; + return true; } -static void +static bool prompt3_hook(const char *newval) { pset.prompt3 = newval ? newval : ""; + return true; } -static void +static bool verbosity_hook(const char *newval) { if (newval == NULL) @@ -942,16 +991,16 @@ verbosity_hook(const char *newval) pset.verbosity = PQERRORS_VERBOSE; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "VERBOSITY", "default"); - pset.verbosity = PQERRORS_DEFAULT; + PsqlVarEnumError("VERBOSITY", newval, "default, terse, verbose"); + return false; } if (pset.db) PQsetErrorVerbosity(pset.db, pset.verbosity); + return true; } -static void +static bool show_context_hook(const char *newval) { if (newval == NULL) @@ -964,13 +1013,13 @@ show_context_hook(const char *newval) pset.show_context = PQSHOW_CONTEXT_ALWAYS; else { - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - newval, "SHOW_CONTEXT", "errors"); - pset.show_context = PQSHOW_CONTEXT_ERRORS; + PsqlVarEnumError("SHOW_CONTEXT", newval, "never, errors, always"); + return false; } if (pset.db) PQsetErrorContextVisibility(pset.db, pset.show_context); + return true; } diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 2669572..d0de198 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -79,92 +79,103 @@ GetVariable(VariableSpace space, const char *name) } /* - * Try to interpret "value" as boolean value. + * Try to interpret "value" as boolean value, and if successful, + * put it in *result. Otherwise don't clobber *result. * * Valid values are: true, false, yes, no, on, off, 1, 0; as well as unique * prefixes thereof. * * "name" is the name of the variable we're assigning to, to use in error * report if any. Pass name == NULL to suppress the error report. + * + * Return true when "value" is syntactically valid, false otherwise. */ bool -ParseVariableBool(const char *value, const char *name) +ParseVariableBool(const char *value, const char *name, bool *result) { size_t len; + bool valid = true; if (value == NULL) - return false; /* not set -> assume "off" */ + { + *result = false; /* not set -> assume "off" */ + return valid; + } len = strlen(value); - if (pg_strncasecmp(value, "true", len) == 0) - return true; - else if (pg_strncasecmp(value, "false", len) == 0) - return false; - else if (pg_strncasecmp(value, "yes", len) == 0) - return true; - else if (pg_strncasecmp(value, "no", len) == 0) - return false; + if (len > 0 && pg_strncasecmp(value, "true", len) == 0) + *result = true; + else if (len > 0 && pg_strncasecmp(value, "false", len) == 0) + *result = false; + else if (len > 0 && pg_strncasecmp(value, "yes", len) == 0) + *result = true; + else if (len > 0 && pg_strncasecmp(value, "no", len) == 0) + *result = false; /* 'o' is not unique enough */ else if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0) - return true; + *result = true; else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0) - return false; + *result = false; else if (pg_strcasecmp(value, "1") == 0) - return true; + *result = true; else if (pg_strcasecmp(value, "0") == 0) - return false; + *result = false; else { - /* NULL is treated as false, so a non-matching value is 'true' */ + /* + * The string is not recognized. We don't clobber *result in this case. + */ if (name) - psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", - value, name, "on"); - return true; + psql_error("unrecognized value \"%s\" for \"%s\": boolean expected\n", + value, name); + valid = false; } + return valid; } /* - * Read numeric variable, or defaultval if it is not set, or faultval if its - * value is not a valid numeric string. If allowtrail is false, this will - * include the case where there are trailing characters after the number. + * Parse a numeric variable from its string representation. + * If the syntax is valid, return true and set *result to the value. + * Otherwise return false and leave *result unchanged. */ -int -ParseVariableNum(const char *val, - int defaultval, - int faultval, - bool allowtrail) +bool +ParseVariableNum(const char *val, int *result) { - int result; + char *end; + int numval; - if (!val) - result = defaultval; - else if (!val[0]) - result = faultval; - else + if (!val || !val[0]) + return false; + errno = 0; + numval = strtol(val, &end, 0); + if (errno == 0 && *end == '\0') { - char *end; - - result = strtol(val, &end, 0); - if (!allowtrail && *end) - result = faultval; + *result = numval; + return true; } - - return result; + return false; } + int GetVariableNum(VariableSpace space, const char *name, int defaultval, - int faultval, - bool allowtrail) + int faultval) { const char *val; + int result; val = GetVariable(space, name); - return ParseVariableNum(val, defaultval, faultval, allowtrail); + if (!val) + return defaultval; + + if (ParseVariableNum(val, &result)) + return result; + else + return faultval; } void @@ -205,13 +216,26 @@ SetVariable(VariableSpace space, const char *name, const char *value) { if (strcmp(current->name, name) == 0) { - /* found entry, so update */ - if (current->value) - free(current->value); - current->value = pg_strdup(value); - if (current->assign_hook) - (*current->assign_hook) (current->value); - return true; + /* + * Found entry, so update, unless a hook returns false. + * The hook needs the value in a buffer with the same lifespan + * as the variable, so allocate it right away, even if it needs + * to be freed just after when the hook returns false. + */ + char *new_value = pg_strdup(value); + + bool confirmed = current->assign_hook ? + (*current->assign_hook) (new_value) : true; + + if (confirmed) + { + pg_free(current->value); + current->value = new_value; + } + else + pg_free(new_value); /* and current->value is left unchanged */ + + return confirmed; } } @@ -248,8 +272,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook { /* found entry, so update */ current->assign_hook = hook; - (*hook) (current->value); - return true; + return (*hook) (current->value); } } @@ -260,8 +283,7 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook current->assign_hook = hook; current->next = NULL; previous->next = current; - (*hook) (NULL); - return true; + return (*hook) (NULL); } bool @@ -303,3 +325,14 @@ DeleteVariable(VariableSpace space, const char *name) return true; } + +/* + * Emit error with suggestions for variables or commands + * accepting enum-style arguments. + */ +void +PsqlVarEnumError(const char* name, const char* value, const char* suggestions) +{ + psql_error("Unrecognized value \"%s\" for \"%s\"\nAvailable values: %s\n", + value, name, suggestions); +} diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h index d235b17..c7d99d1 100644 --- a/src/bin/psql/variables.h +++ b/src/bin/psql/variables.h @@ -12,15 +12,19 @@ * This implements a sort of variable repository. One could also think of it * as a cheap version of an associative array. In each one of these * datastructures you can store name/value pairs. There can also be an - * "assign hook" function that is called whenever the variable's value is - * changed. + * "assign hook" function that is called before the variable's value is + * changed, and that returns a boolean indicating whether the assignment + * is confirmed or must be cancelled. * - * An "unset" operation causes the hook to be called with newval == NULL. + * The hook is called with newval == NULL when a variable is created + * along with it by SetVariableAssignHook(). + * An "unset" operation also causes the hook to be called with newval == NULL, + * and is not cancellable even if the hook returns false. * * Note: if value == NULL then the variable is logically unset, but we are * keeping the struct around so as not to forget about its hook function. */ -typedef void (*VariableAssignHook) (const char *newval); +typedef bool (*VariableAssignHook) (const char *newval); struct _variable { @@ -35,16 +39,15 @@ typedef struct _variable *VariableSpace; VariableSpace CreateVariableSpace(void); const char *GetVariable(VariableSpace space, const char *name); -bool ParseVariableBool(const char *value, const char *name); -int ParseVariableNum(const char *val, - int defaultval, - int faultval, - bool allowtrail); +bool ParseVariableBool(const char *value, const char *name, bool *result); + +bool ParseVariableNum(const char *val, int* result); + int GetVariableNum(VariableSpace space, const char *name, int defaultval, - int faultval, - bool allowtrail); + int faultval); + void PrintVariables(VariableSpace space); @@ -53,4 +56,6 @@ bool SetVariableAssignHook(VariableSpace space, const char *name, VariableAssig bool SetVariableBool(VariableSpace space, const char *name); bool DeleteVariable(VariableSpace space, const char *name); +void PsqlVarEnumError(const char* name, const char* value, const char* suggestions); + #endif /* VARIABLES_H */