From 28551797a46c0ada035c34cf9cf2bc90448e3c26 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 31 Dec 2014 12:16:53 -0500 Subject: [PATCH] Improve consistency of parsing of psql's magic variables. For simple boolean variables such as ON_ERROR_STOP, psql has for a long time recognized variant spellings of "on" and "off" (such as "1"/"0"), and it also made a point of warning you if you'd misspelled the setting. But these conveniences did not exist for other keyword-valued variables. In particular, though ECHO_HIDDEN and ON_ERROR_ROLLBACK include "on" and "off" as possible values, none of the alternative spellings for those were recognized; and to make matters worse the code would just silently assume "on" was meant for any unrecognized spelling. Several people have reported getting bitten by this, so let's fix it. In detail, this patch: * Allows all spellings recognized by ParseVariableBool() for ECHO_HIDDEN and ON_ERROR_ROLLBACK. * Reports a warning for unrecognized values for COMP_KEYWORD_CASE, ECHO, ECHO_HIDDEN, HISTCONTROL, ON_ERROR_ROLLBACK, and VERBOSITY. * Recognizes all values for all these variables case-insensitively; previously there was a mishmash of case-sensitive and case-insensitive behaviors. Back-patch to all supported branches. There is a small risk of breaking existing scripts that were accidentally failing to malfunction; but the consensus is that the chance of detecting real problems and preventing future mistakes outweighs this. --- doc/src/sgml/ref/psql-ref.sgml | 39 ++++++++-------- src/bin/psql/command.c | 18 +++++--- src/bin/psql/settings.h | 14 ++++++ src/bin/psql/startup.c | 84 ++++++++++++++++++++++++---------- src/bin/psql/tab-complete.c | 34 ++++---------- src/bin/psql/variables.c | 15 ++++-- src/bin/psql/variables.h | 2 +- 7 files changed, 127 insertions(+), 79 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index d29dfa2b87..bdfb67cd9a 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -173,8 +173,7 @@ EOF Echo the actual queries generated by \d and other backslash commands. You can use this to study psql's internal operations. This is equivalent to - setting the variable ECHO_HIDDEN from within - psql. + setting the variable ECHO_HIDDEN to on. @@ -333,8 +332,8 @@ EOF quietly. By default, it prints welcome messages and various informational output. If this option is used, none of this happens. This is useful with the option. - Within psql you can also set the - QUIET variable to achieve the same effect. + This is equivalent to setting the variable QUIET + to on. @@ -2884,8 +2883,9 @@ bar ECHO_HIDDEN - When this variable is set and a backslash command queries the - database, the query is first shown. This way you can study the + When this variable is set to on and a backslash command + queries the database, the query is first shown. + This feature helps you to study PostgreSQL internals and provide similar functionality in your own programs. (To select this behavior on program start-up, use the switch .) If you set @@ -3046,16 +3046,16 @@ bar - When on, if a statement in a transaction block + When set to on, if a statement in a transaction block generates an error, the error is ignored and the transaction - continues. When interactive, such errors are only + continues. When set to interactive, such errors are only ignored in interactive sessions, and not when reading script - files. When off (the default), a statement in a + files. When unset or set to off, a statement in a transaction block that generates an error aborts the entire - transaction. The on_error_rollback-on mode works by issuing an + transaction. The error rollback mode works by issuing an implicit SAVEPOINT for you, just before each command - that is in a transaction block, and rolls back to the savepoint - on error. + that is in a transaction block, and then rolling back to the + savepoint if the command fails. @@ -3065,7 +3065,8 @@ bar By default, command processing continues after an error. When this - variable is set, it will instead stop immediately. In interactive mode, + variable is set to on, processing will instead stop + immediately. In interactive mode, psql will return to the command prompt; otherwise, psql will exit, returning error code 3 to distinguish this case from fatal error @@ -3107,8 +3108,8 @@ bar QUIET - This variable is equivalent to the command line option - . It is probably not too useful in + Setting this variable to on is equivalent to the command + line option . It is probably not too useful in interactive mode. @@ -3118,8 +3119,8 @@ bar SINGLELINE - This variable is equivalent to the command line option - . + Setting this variable to on is equivalent to the command + line option . @@ -3128,8 +3129,8 @@ bar SINGLESTEP - This variable is equivalent to the command line option - . + Setting this variable to on is equivalent to the command + line option . diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index c7a17d7469..504cc0d03c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1362,7 +1362,7 @@ exec_command(const char *cmd, OT_NORMAL, NULL, false); if (opt) - pset.timing = ParseVariableBool(opt); + pset.timing = ParseVariableBool(opt, "\\timing"); else pset.timing = !pset.timing; if (!pset.quiet) @@ -2412,12 +2412,14 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) } /* set expanded/vertical mode */ - else if (strcmp(param, "x") == 0 || strcmp(param, "expanded") == 0 || strcmp(param, "vertical") == 0) + else if (strcmp(param, "x") == 0 || + strcmp(param, "expanded") == 0 || + strcmp(param, "vertical") == 0) { if (value && pg_strcasecmp(value, "auto") == 0) popt->topt.expanded = 2; else if (value) - popt->topt.expanded = ParseVariableBool(value); + popt->topt.expanded = ParseVariableBool(value, param); else popt->topt.expanded = !popt->topt.expanded; } @@ -2426,7 +2428,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); + popt->topt.numericLocale = ParseVariableBool(value, param); else popt->topt.numericLocale = !popt->topt.numericLocale; } @@ -2481,7 +2483,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); + popt->topt.tuples_only = ParseVariableBool(value, param); else popt->topt.tuples_only = !popt->topt.tuples_only; } @@ -2512,10 +2514,12 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) if (value && pg_strcasecmp(value, "always") == 0) popt->topt.pager = 2; else if (value) - if (ParseVariableBool(value)) + { + if (ParseVariableBool(value, param)) popt->topt.pager = 1; else popt->topt.pager = 0; + } else if (popt->topt.pager == 1) popt->topt.pager = 0; else @@ -2526,7 +2530,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); + popt->topt.default_footer = ParseVariableBool(value, param); else popt->topt.default_footer = !popt->topt.default_footer; } diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h index ef24a4ef98..bad70e4c8a 100644 --- a/src/bin/psql/settings.h +++ b/src/bin/psql/settings.h @@ -27,6 +27,11 @@ #define DEFAULT_PROMPT2 "%/%R%# " #define DEFAULT_PROMPT3 ">> " +/* + * Note: these enums should generally be chosen so that zero corresponds + * to the default behavior. + */ + typedef enum { PSQL_ECHO_NONE, @@ -49,6 +54,14 @@ typedef enum PSQL_ERROR_ROLLBACK_ON } PSQL_ERROR_ROLLBACK; +typedef enum +{ + PSQL_COMP_CASE_PRESERVE_UPPER, + PSQL_COMP_CASE_PRESERVE_LOWER, + PSQL_COMP_CASE_UPPER, + PSQL_COMP_CASE_LOWER +} PSQL_COMP_CASE; + typedef enum { hctl_none = 0, @@ -110,6 +123,7 @@ typedef struct _psqlSettings PSQL_ECHO echo; PSQL_ECHO_HIDDEN echo_hidden; PSQL_ERROR_ROLLBACK on_error_rollback; + PSQL_COMP_CASE comp_case; HistControl histcontrol; const char *prompt1; const char *prompt2; diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 11a159a164..a23ebce9bf 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -705,31 +705,31 @@ showVersion(void) static void autocommit_hook(const char *newval) { - pset.autocommit = ParseVariableBool(newval); + pset.autocommit = ParseVariableBool(newval, "AUTOCOMMIT"); } static void on_error_stop_hook(const char *newval) { - pset.on_error_stop = ParseVariableBool(newval); + pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP"); } static void quiet_hook(const char *newval) { - pset.quiet = ParseVariableBool(newval); + pset.quiet = ParseVariableBool(newval, "QUIET"); } static void singleline_hook(const char *newval) { - pset.singleline = ParseVariableBool(newval); + pset.singleline = ParseVariableBool(newval, "SINGLELINE"); } static void singlestep_hook(const char *newval) { - pset.singlestep = ParseVariableBool(newval); + pset.singlestep = ParseVariableBool(newval, "SINGLESTEP"); } static void @@ -743,14 +743,20 @@ echo_hook(const char *newval) { if (newval == NULL) pset.echo = PSQL_ECHO_NONE; - else if (strcmp(newval, "queries") == 0) + else if (pg_strcasecmp(newval, "queries") == 0) pset.echo = PSQL_ECHO_QUERIES; - else if (strcmp(newval, "errors") == 0) + else if (pg_strcasecmp(newval, "errors") == 0) pset.echo = PSQL_ECHO_ERRORS; - else if (strcmp(newval, "all") == 0) + else if (pg_strcasecmp(newval, "all") == 0) pset.echo = PSQL_ECHO_ALL; - else + else if (pg_strcasecmp(newval, "none") == 0) pset.echo = PSQL_ECHO_NONE; + else + { + psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", + newval, "ECHO", "none"); + pset.echo = PSQL_ECHO_NONE; + } } static void @@ -758,12 +764,12 @@ echo_hidden_hook(const char *newval) { if (newval == NULL) pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; - else if (strcmp(newval, "noexec") == 0) + else if (pg_strcasecmp(newval, "noexec") == 0) pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC; - else if (pg_strcasecmp(newval, "off") == 0) - pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; - else + 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; } static void @@ -773,10 +779,31 @@ on_error_rollback_hook(const char *newval) 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 (pg_strcasecmp(newval, "off") == 0) - pset.on_error_rollback = PSQL_ERROR_ROLLBACK_OFF; - else + 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; +} + +static void +comp_keyword_case_hook(const char *newval) +{ + if (newval == NULL) + pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER; + else if (pg_strcasecmp(newval, "preserve-upper") == 0) + pset.comp_case = PSQL_COMP_CASE_PRESERVE_UPPER; + else if (pg_strcasecmp(newval, "preserve-lower") == 0) + pset.comp_case = PSQL_COMP_CASE_PRESERVE_LOWER; + else if (pg_strcasecmp(newval, "upper") == 0) + pset.comp_case = PSQL_COMP_CASE_UPPER; + else if (pg_strcasecmp(newval, "lower") == 0) + 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; + } } static void @@ -784,14 +811,20 @@ histcontrol_hook(const char *newval) { if (newval == NULL) pset.histcontrol = hctl_none; - else if (strcmp(newval, "ignorespace") == 0) + else if (pg_strcasecmp(newval, "ignorespace") == 0) pset.histcontrol = hctl_ignorespace; - else if (strcmp(newval, "ignoredups") == 0) + else if (pg_strcasecmp(newval, "ignoredups") == 0) pset.histcontrol = hctl_ignoredups; - else if (strcmp(newval, "ignoreboth") == 0) + else if (pg_strcasecmp(newval, "ignoreboth") == 0) pset.histcontrol = hctl_ignoreboth; - else + else if (pg_strcasecmp(newval, "none") == 0) pset.histcontrol = hctl_none; + else + { + psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", + newval, "HISTCONTROL", "none"); + pset.histcontrol = hctl_none; + } } static void @@ -817,14 +850,18 @@ verbosity_hook(const char *newval) { if (newval == NULL) pset.verbosity = PQERRORS_DEFAULT; - else if (strcmp(newval, "default") == 0) + else if (pg_strcasecmp(newval, "default") == 0) pset.verbosity = PQERRORS_DEFAULT; - else if (strcmp(newval, "terse") == 0) + else if (pg_strcasecmp(newval, "terse") == 0) pset.verbosity = PQERRORS_TERSE; - else if (strcmp(newval, "verbose") == 0) + else if (pg_strcasecmp(newval, "verbose") == 0) pset.verbosity = PQERRORS_VERBOSE; else + { + psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", + newval, "VERBOSITY", "default"); pset.verbosity = PQERRORS_DEFAULT; + } if (pset.db) PQsetErrorVerbosity(pset.db, pset.verbosity); @@ -845,6 +882,7 @@ EstablishVariableSpace(void) SetVariableAssignHook(pset.vars, "ECHO", echo_hook); SetVariableAssignHook(pset.vars, "ECHO_HIDDEN", echo_hidden_hook); SetVariableAssignHook(pset.vars, "ON_ERROR_ROLLBACK", on_error_rollback_hook); + SetVariableAssignHook(pset.vars, "COMP_KEYWORD_CASE", comp_keyword_case_hook); SetVariableAssignHook(pset.vars, "HISTCONTROL", histcontrol_hook); SetVariableAssignHook(pset.vars, "PROMPT1", prompt1_hook); SetVariableAssignHook(pset.vars, "PROMPT2", prompt2_hook); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 7212015cf0..4f586302bd 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -4453,7 +4453,7 @@ complete_from_files(const char *text, int state) /* * Make a pg_strdup copy of s and convert the case according to - * COMP_KEYWORD_CASE variable, using ref as the text that was already entered. + * COMP_KEYWORD_CASE setting, using ref as the text that was already entered. */ static char * pg_strdup_keyword_case(const char *s, const char *ref) @@ -4461,38 +4461,22 @@ pg_strdup_keyword_case(const char *s, const char *ref) char *ret, *p; unsigned char first = ref[0]; - int tocase; - const char *varval; - - varval = GetVariable(pset.vars, "COMP_KEYWORD_CASE"); - if (!varval) - tocase = 0; - else if (strcmp(varval, "lower") == 0) - tocase = -2; - else if (strcmp(varval, "preserve-lower") == 0) - tocase = -1; - else if (strcmp(varval, "preserve-upper") == 0) - tocase = +1; - else if (strcmp(varval, "upper") == 0) - tocase = +2; - else - tocase = 0; - - /* default */ - if (tocase == 0) - tocase = +1; ret = pg_strdup(s); - if (tocase == -2 - || ((tocase == -1 || tocase == +1) && islower(first)) - || (tocase == -1 && !isalpha(first)) - ) + if (pset.comp_case == PSQL_COMP_CASE_LOWER || + ((pset.comp_case == PSQL_COMP_CASE_PRESERVE_LOWER || + pset.comp_case == PSQL_COMP_CASE_PRESERVE_UPPER) && islower(first)) || + (pset.comp_case == PSQL_COMP_CASE_PRESERVE_LOWER && !isalpha(first))) + { for (p = ret; *p; p++) *p = pg_tolower((unsigned char) *p); + } else + { for (p = ret; *p; p++) *p = pg_toupper((unsigned char) *p); + } return ret; } diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 1f871d7fa0..29c50f969c 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -79,11 +79,16 @@ GetVariable(VariableSpace space, const char *name) } /* - * Try to interpret value as boolean value. Valid values are: true, - * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof. + * Try to interpret "value" as boolean value. + * + * 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. */ bool -ParseVariableBool(const char *value) +ParseVariableBool(const char *value, const char *name) { size_t len; @@ -112,7 +117,9 @@ ParseVariableBool(const char *value) else { /* NULL is treated as false, so a non-matching value is 'true' */ - psql_error("unrecognized Boolean value; assuming \"on\"\n"); + if (name) + psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", + value, name, "on"); return true; } } diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h index 5a2b98e122..f94f779624 100644 --- a/src/bin/psql/variables.h +++ b/src/bin/psql/variables.h @@ -35,7 +35,7 @@ typedef struct _variable *VariableSpace; VariableSpace CreateVariableSpace(void); const char *GetVariable(VariableSpace space, const char *name); -bool ParseVariableBool(const char *val); +bool ParseVariableBool(const char *value, const char *name); int ParseVariableNum(const char *val, int defaultval, int faultval,