Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cYfeW-0002yN-5P for pgsql-hackers@arkaria.postgresql.org; Tue, 31 Jan 2017 21:07:48 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1cYfeV-0007H8-LZ for pgsql-hackers@arkaria.postgresql.org; Tue, 31 Jan 2017 21:07:47 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1cYfeR-0007EU-SC for pgsql-hackers@postgresql.org; Tue, 31 Jan 2017 21:07:44 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1cYfeM-0002gk-U6 for pgsql-hackers@postgresql.org; Tue, 31 Jan 2017 21:07:43 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id v0VL7C1T009832; Tue, 31 Jan 2017 16:07:12 -0500 From: Tom Lane To: "Daniel Verite" cc: "Rahila Syed" , "Stephen Frost" , "Ashutosh Bapat" , "pgsql-hackers" , "Andrew Dunstan" Subject: Re: Improvements in psql hooks for variables In-reply-to: <30629.1485881533@sss.pgh.pa.us> References: <0451dd1e-aad1-4f9c-855e-6cc6584d4b54@manitou-mail.org> <30629.1485881533@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Tue, 31 Jan 2017 11:52:13 -0500" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <9798.1485896794.0@sss.pgh.pa.us> Date: Tue, 31 Jan 2017 16:07:12 -0500 Message-ID: <9831.1485896832@sss.pgh.pa.us> X-Pg-Spam-Score: -1.9 (-) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <9798.1485896794.1@sss.pgh.pa.us> I wrote: > Attached is a draft patch for that. I chose to make a second hook rather > than complicate the assign hook API, mainly because it allows more code > sharing --- all the bool vars can share the same substitute hook, and > so can the three-way vars as long as "on" and "off" are the appropriate > substitutes. > I only touched the behavior for bool vars here, but if people like this > solution it could be fleshed out to apply to all the built-in variables. Attached is a finished version that includes hooks for all the variables for which they were clearly sensible. (FETCH_COUNT doesn't seem to really need one, and I didn't do anything with HISTSIZE or IGNOREEOF either. It might be worth bringing the latter two into the hooks paradigm, but that seems like fit material for a separate patch.) I updated the documentation as well. I think this is committable if there are not objections. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="improve-psql-bool-var-behavior-2.patch"; charset="us-ascii" Content-ID: <9798.1485896794.2@sss.pgh.pa.us> Content-Description: improve-psql-bool-var-behavior-2.patch Content-Transfer-Encoding: quoted-printable diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 4e51e90..b9c8fcc 100644 *** a/doc/src/sgml/ref/psql-ref.sgml --- b/doc/src/sgml/ref/psql-ref.sgml *************** EOF *** 455,462 **** any, by an equal sign on the command line. To unset a variable, leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are ! done during a very early stage of start-up, so variables reserved ! for internal purposes might get overwritten later. --- 455,462 ---- any, by an equal sign on the command line. To unset a variable, leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are ! done during command line processing, so variables that reflect ! connection state will get overwritten later. *************** lo_import 152801 *** 2692,2698 **** class=3D"parameter">name to value, or if more than one value is given, to the concatenation of all of them. If only one ! argument is given, the variable is set with an empty value. To unset a variable, use the \unset command. =20=20 --- 2692,2698 ---- class=3D"parameter">name to value, or if more than one value is given, to the concatenation of all of them. If only one ! argument is given, the variable is set to an empty-string value. = To unset a variable, use the \unset command. =20=20 *************** lo_import 152801 *** 2709,2717 **** =20=20 ! Although you are welcome to set any variable to anything you ! want, psql treats several variables ! as special. They are documented in the section about variables. =20=20 --- 2709,2719 ---- =20=20 ! Certain variables are special, in that they ! control psql's behavior or are ! automatically set to reflect connection state. These variables a= re ! documented in , below. =20=20 *************** testdb=3D> \setenv LESS -imx *** 2835,2840 **** --- 2837,2850 ---- Unsets (deletes) the psql variable name. +=20 + + Most variables that control psql's beh= avior + cannot be unset; instead, an \unset command is interp= reted + as setting them to their default values. + See , below. + =20=20 *************** bar *** 3053,3059 **** =20=20 If you call \set without a second argument, the ! variable is set, with an empty string as value. To unset (i.e., delet= e) a variable, use the command \unset. To show the values of all variables, call \set without any arg= ument. --- 3063,3069 ---- =20=20 If you call \set without a second argument, the ! variable is set to an empty-string value. To unset (i.e., delete) a variable, use the command \unset. To show the values of all variables, call \set without any arg= ument. *************** bar *** 3082,3089 **** By convention, all specially treated variables' names consist of all upper-case ASCII letters (and possibly digits and underscores). To ensure maximum compatibility in the future, avoid ! using such variable names for your own purposes. A list of all specia= lly ! treated variables follows. =20=20 --- 3092,3114 ---- By convention, all specially treated variables' names consist of all upper-case ASCII letters (and possibly digits and underscores). To ensure maximum compatibility in the future, avoid ! using such variable names for your own purposes. ! !=20 ! ! Variables that control psql's behavior ! generally cannot be unset or set to invalid values. An \uns= et ! command is allowed but is interpreted as setting the variable to its ! default value. A \set command without a second argument = is ! interpreted as setting the variable to on, for control ! variables that accept that value, and is rejected for others. Also, ! control variables that accept the values on ! and off will also accept other common spellings of Boolean ! values, such as true and false. ! !=20 ! ! The specially treated variables are: =20=20 *************** bar *** 3153,3159 **** The name of the database you are currently connected to. This is set every time you connect to a database (including program ! start-up), but can be unset. --- 3178,3184 ---- The name of the database you are currently connected to. This is set every time you connect to a database (including program ! start-up), but can be changed or unset. *************** bar *** 3171,3178 **** as it is sent to the server. The switch to select this behavior is . If set to errors, then on= ly failed queries are displayed on standard error output. The switch ! for this behavior is . If unset, or if set to ! none, then no queries are displayed. --- 3196,3203 ---- as it is sent to the server. The switch to select this behavior is . If set to errors, then on= ly failed queries are displayed on standard error output. The switch ! for this behavior is . If set to ! none (the default), then no queries are displa= yed. *************** bar *** 3187,3194 **** PostgreSQL internals and provide similar functionality in your own programs. (To select this behav= ior on program start-up, use the switch .) If you= set ! the variable to the value noexec, the queries = are just shown but are not actually sent to the server and executed. --- 3212,3220 ---- PostgreSQL internals and provide similar functionality in your own programs. (To select this behav= ior on program start-up, use the switch .) If you= set ! this variable to the value noexec, the queries= are just shown but are not actually sent to the server and executed. + The default value is off. *************** bar *** 3200,3206 **** The current client character set encoding. This is set every time you connect to a database (including program start-up), and when you change the encoding ! with \encoding, but it can be unset. --- 3226,3232 ---- The current client character set encoding. This is set every time you connect to a database (including program start-up), and when you change the encoding ! with \encoding, but it can be changed or unset. *************** bar *** 3209,3215 **** FETCH_COUNT ! If this variable is set to an integer value > 0, the results of SELECT queries are fetched and displayed in groups of that many rows, rather than the default behavior of collecting the entire result set before --- 3235,3241 ---- FETCH_COUNT ! If this variable is set to an integer value greater than zero, the results of SELECT queries are fetched and displayed in groups of that many rows, rather than the default behavior of collecting the entire result set before *************** bar *** 3220,3225 **** --- 3246,3258 ---- Keep in mind that when using this feature, a query might fail after having already displayed some rows. +=20 + + FETCH_COUNT is ignored if it is unset or does = not + have a positive value. It cannot be set to a value that is not + syntactically an integer. + +=20 Although you can use any output format with this feature, *************** bar *** 3241,3247 **** 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 (the default), all l= ines read in interactive mode are saved on the history list. --- 3274,3280 ---- 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 ! set to none (the default), all lines read in interactive mode are saved on the history list. *************** bar *** 3257,3264 **** HISTFILE ! The file name that will be used to store the history list. The de= fault ! value is ~/.psql_history. For example, putt= ing: \set HISTFILE ~/.psql_history- :DBNAME --- 3290,3301 ---- HISTFILE ! The file name that will be used to store the history list. If un= set, ! the file name is taken from the PSQL_HISTORY ! environment variable. If that is not set either, the default ! is ~/.psql_history, ! or %APPDATA%\postgresql\psql_history on Wind= ows. ! For example, putting: \set HISTFILE ~/.psql_history- :DBNAME *************** bar *** 3279,3286 **** HISTSIZE ! The number of commands to store in the command history. The ! default value is 500. --- 3316,3325 ---- HISTSIZE ! The maximum number of commands to store in the command history. ! If unset, at most 500 commands are stored by default. ! If set to a value that is negative or not an integer, no limit is ! applied. *************** bar *** 3297,3303 **** The database server host you are currently connected to. This is set every time you connect to a database (including program ! start-up), but can be unset. --- 3336,3342 ---- The database server host you are currently connected to. This is set every time you connect to a database (including program ! start-up), but can be changed or unset. *************** bar *** 3350,3356 **** generates an error, the error is ignored and the transaction continues. When set to interactive, such errors are o= nly ignored in interactive sessions, and not when reading script ! files. When unset or set to off, a statement in a transaction block that generates an error aborts the entire transaction. The error rollback mode works by issuing an implicit SAVEPOINT for you, just before each command --- 3389,3395 ---- generates an error, the error is ignored and the transaction continues. When set to interactive, such errors are o= nly ignored in interactive sessions, and not when reading script ! files. When set to off (the default), a statement in a transaction block that generates an error aborts the entire transaction. The error rollback mode works by issuing an implicit SAVEPOINT for you, just before each command *************** bar *** 3385,3391 **** The database server port to which you are currently connected. This is set every time you connect to a database (including ! program start-up), but can be unset. --- 3424,3430 ---- The database server port to which you are currently connected. This is set every time you connect to a database (including ! program start-up), but can be changed or unset. *************** bar *** 3458,3464 **** The database user you are currently connected as. This is set every time you connect to a database (including program ! start-up), but can be unset. --- 3497,3503 ---- The database user you are currently connected as. This is set every time you connect to a database (including program ! start-up), but can be changed or unset. *************** bar *** 3481,3487 **** This variable is set at program start-up to ! reflect psql's version. It can be unset or chang= ed. --- 3520,3526 ---- This variable is set at program start-up to ! reflect psql's version. It can be changed or uns= et. *************** PSQL_EDITOR_LINENUMBER_ARG=3D'--line ' *** 4015,4020 **** --- 4054,4060 ---- The location of the history file can be set explicitly via + the HISTFILE psql variable or the PSQL_HISTORY environment variable. diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 0574b5b..a3654e6 100644 *** a/src/bin/psql/startup.c --- b/src/bin/psql/startup.c *************** main(int argc, char *argv[]) *** 166,175 **** =20=20 SetVariable(pset.vars, "VERSION", PG_VERSION_STR); =20=20 ! /* Default values for variables */ SetVariableBool(pset.vars, "AUTOCOMMIT"); - SetVariable(pset.vars, "VERBOSITY", "default"); - SetVariable(pset.vars, "SHOW_CONTEXT", "errors"); SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1); SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2); SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3); --- 166,173 ---- =20=20 SetVariable(pset.vars, "VERSION", PG_VERSION_STR); =20=20 ! /* Default values for variables (that don't match the result of \unset) = */ SetVariableBool(pset.vars, "AUTOCOMMIT"); SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1); SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2); SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3); *************** parse_psql_options(int argc, char *argv[ *** 578,594 **** if (!equal_loc) { if (!DeleteVariable(pset.vars, value)) ! { ! fprintf(stderr, _("%s: could not delete variable \"%s\"\n"), ! pset.progname, value); ! exit(EXIT_FAILURE); ! } } else { *equal_loc =3D '\0'; if (!SetVariable(pset.vars, value, equal_loc + 1)) ! exit(EXIT_FAILURE); } =20=20 free(value); --- 576,588 ---- if (!equal_loc) { if (!DeleteVariable(pset.vars, value)) ! exit(EXIT_FAILURE); /* error already printed */ } else { *equal_loc =3D '\0'; if (!SetVariable(pset.vars, value, equal_loc + 1)) ! exit(EXIT_FAILURE); /* error already printed */ } =20=20 free(value); *************** showVersion(void) *** 777,787 **** =20=20 =20=20 /* ! * Assign hooks for psql variables. * * This isn't an amazingly good place for them, but neither is anywhere e= lse. */ =20=20 static bool autocommit_hook(const char *newval) { --- 771,798 ---- =20=20 =20=20 /* ! * Substitute hooks and assign hooks for psql variables. * * This isn't an amazingly good place for them, but neither is anywhere e= lse. */ =20=20 + static char * + bool_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + { + /* "\unset FOO" becomes "\set FOO off" */ + newval =3D pg_strdup("off"); + } + else if (newval[0] =3D=3D '\0') + { + /* "\set FOO" becomes "\set FOO on" */ + pg_free(newval); + newval =3D pg_strdup("on"); + } + return newval; + } +=20 static bool autocommit_hook(const char *newval) { *************** fetch_count_hook(const char *newval) *** 822,833 **** return true; } =20=20 static bool echo_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.echo =3D PSQL_ECHO_NONE; ! else if (pg_strcasecmp(newval, "queries") =3D=3D 0) pset.echo =3D PSQL_ECHO_QUERIES; else if (pg_strcasecmp(newval, "errors") =3D=3D 0) pset.echo =3D PSQL_ECHO_ERRORS; --- 833,851 ---- return true; } =20=20 + static char * + echo_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + newval =3D pg_strdup("none"); + return newval; + } +=20 static bool echo_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "queries") =3D=3D 0) pset.echo =3D PSQL_ECHO_QUERIES; else if (pg_strcasecmp(newval, "errors") =3D=3D 0) pset.echo =3D PSQL_ECHO_ERRORS; *************** echo_hook(const char *newval) *** 846,854 **** static bool echo_hidden_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.echo_hidden =3D PSQL_ECHO_HIDDEN_OFF; ! else if (pg_strcasecmp(newval, "noexec") =3D=3D 0) pset.echo_hidden =3D PSQL_ECHO_HIDDEN_NOEXEC; else { --- 864,871 ---- static bool echo_hidden_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "noexec") =3D=3D 0) pset.echo_hidden =3D PSQL_ECHO_HIDDEN_NOEXEC; else { *************** echo_hidden_hook(const char *newval) *** 868,876 **** static bool on_error_rollback_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.on_error_rollback =3D PSQL_ERROR_ROLLBACK_OFF; ! else if (pg_strcasecmp(newval, "interactive") =3D=3D 0) pset.on_error_rollback =3D PSQL_ERROR_ROLLBACK_INTERACTIVE; else { --- 885,892 ---- static bool on_error_rollback_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "interactive") =3D=3D 0) pset.on_error_rollback =3D PSQL_ERROR_ROLLBACK_INTERACTIVE; else { *************** on_error_rollback_hook(const char *newva *** 887,898 **** return true; } =20=20 static bool comp_keyword_case_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.comp_case =3D PSQL_COMP_CASE_PRESERVE_UPPER; ! else if (pg_strcasecmp(newval, "preserve-upper") =3D=3D 0) pset.comp_case =3D PSQL_COMP_CASE_PRESERVE_UPPER; else if (pg_strcasecmp(newval, "preserve-lower") =3D=3D 0) pset.comp_case =3D PSQL_COMP_CASE_PRESERVE_LOWER; --- 903,921 ---- return true; } =20=20 + static char * + comp_keyword_case_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + newval =3D pg_strdup("preserve-upper"); + return newval; + } +=20 static bool comp_keyword_case_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "preserve-upper") =3D=3D 0) pset.comp_case =3D PSQL_COMP_CASE_PRESERVE_UPPER; else if (pg_strcasecmp(newval, "preserve-lower") =3D=3D 0) pset.comp_case =3D PSQL_COMP_CASE_PRESERVE_LOWER; *************** comp_keyword_case_hook(const char *newva *** 909,920 **** return true; } =20=20 static bool histcontrol_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.histcontrol =3D hctl_none; ! else if (pg_strcasecmp(newval, "ignorespace") =3D=3D 0) pset.histcontrol =3D hctl_ignorespace; else if (pg_strcasecmp(newval, "ignoredups") =3D=3D 0) pset.histcontrol =3D hctl_ignoredups; --- 932,950 ---- return true; } =20=20 + static char * + histcontrol_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + newval =3D pg_strdup("none"); + return newval; + } +=20 static bool histcontrol_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "ignorespace") =3D=3D 0) pset.histcontrol =3D hctl_ignorespace; else if (pg_strcasecmp(newval, "ignoredups") =3D=3D 0) pset.histcontrol =3D hctl_ignoredups; *************** prompt3_hook(const char *newval) *** 952,963 **** return true; } =20=20 static bool verbosity_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.verbosity =3D PQERRORS_DEFAULT; ! else if (pg_strcasecmp(newval, "default") =3D=3D 0) pset.verbosity =3D PQERRORS_DEFAULT; else if (pg_strcasecmp(newval, "terse") =3D=3D 0) pset.verbosity =3D PQERRORS_TERSE; --- 982,1000 ---- return true; } =20=20 + static char * + verbosity_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + newval =3D pg_strdup("default"); + return newval; + } +=20 static bool verbosity_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "default") =3D=3D 0) pset.verbosity =3D PQERRORS_DEFAULT; else if (pg_strcasecmp(newval, "terse") =3D=3D 0) pset.verbosity =3D PQERRORS_TERSE; *************** verbosity_hook(const char *newval) *** 974,985 **** return true; } =20=20 static bool show_context_hook(const char *newval) { ! if (newval =3D=3D NULL) ! pset.show_context =3D PQSHOW_CONTEXT_ERRORS; ! else if (pg_strcasecmp(newval, "never") =3D=3D 0) pset.show_context =3D PQSHOW_CONTEXT_NEVER; else if (pg_strcasecmp(newval, "errors") =3D=3D 0) pset.show_context =3D PQSHOW_CONTEXT_ERRORS; --- 1011,1029 ---- return true; } =20=20 + static char * + show_context_substitute_hook(char *newval) + { + if (newval =3D=3D NULL) + newval =3D pg_strdup("errors"); + return newval; + } +=20 static bool show_context_hook(const char *newval) { ! Assert(newval !=3D NULL); /* else substitute hook messed up */ ! if (pg_strcasecmp(newval, "never") =3D=3D 0) pset.show_context =3D PQSHOW_CONTEXT_NEVER; else if (pg_strcasecmp(newval, "errors") =3D=3D 0) pset.show_context =3D PQSHOW_CONTEXT_ERRORS; *************** EstablishVariableSpace(void) *** 1002,1021 **** { pset.vars =3D CreateVariableSpace(); =20=20 ! SetVariableAssignHook(pset.vars, "AUTOCOMMIT", autocommit_hook); ! SetVariableAssignHook(pset.vars, "ON_ERROR_STOP", on_error_stop_hook); ! SetVariableAssignHook(pset.vars, "QUIET", quiet_hook); ! SetVariableAssignHook(pset.vars, "SINGLELINE", singleline_hook); ! SetVariableAssignHook(pset.vars, "SINGLESTEP", singlestep_hook); ! SetVariableAssignHook(pset.vars, "FETCH_COUNT", fetch_count_hook); ! 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); ! SetVariableAssignHook(pset.vars, "PROMPT3", prompt3_hook); ! SetVariableAssignHook(pset.vars, "VERBOSITY", verbosity_hook); ! SetVariableAssignHook(pset.vars, "SHOW_CONTEXT", show_context_hook); } --- 1046,1097 ---- { pset.vars =3D CreateVariableSpace(); =20=20 ! SetVariableHooks(pset.vars, "AUTOCOMMIT", ! bool_substitute_hook, ! autocommit_hook); ! SetVariableHooks(pset.vars, "ON_ERROR_STOP", ! bool_substitute_hook, ! on_error_stop_hook); ! SetVariableHooks(pset.vars, "QUIET", ! bool_substitute_hook, ! quiet_hook); ! SetVariableHooks(pset.vars, "SINGLELINE", ! bool_substitute_hook, ! singleline_hook); ! SetVariableHooks(pset.vars, "SINGLESTEP", ! bool_substitute_hook, ! singlestep_hook); ! SetVariableHooks(pset.vars, "FETCH_COUNT", ! NULL, ! fetch_count_hook); ! SetVariableHooks(pset.vars, "ECHO", ! echo_substitute_hook, ! echo_hook); ! SetVariableHooks(pset.vars, "ECHO_HIDDEN", ! bool_substitute_hook, ! echo_hidden_hook); ! SetVariableHooks(pset.vars, "ON_ERROR_ROLLBACK", ! bool_substitute_hook, ! on_error_rollback_hook); ! SetVariableHooks(pset.vars, "COMP_KEYWORD_CASE", ! comp_keyword_case_substitute_hook, ! comp_keyword_case_hook); ! SetVariableHooks(pset.vars, "HISTCONTROL", ! histcontrol_substitute_hook, ! histcontrol_hook); ! SetVariableHooks(pset.vars, "PROMPT1", ! NULL, ! prompt1_hook); ! SetVariableHooks(pset.vars, "PROMPT2", ! NULL, ! prompt2_hook); ! SetVariableHooks(pset.vars, "PROMPT3", ! NULL, ! prompt3_hook); ! SetVariableHooks(pset.vars, "VERBOSITY", ! verbosity_substitute_hook, ! verbosity_hook); ! SetVariableHooks(pset.vars, "SHOW_CONTEXT", ! show_context_substitute_hook, ! show_context_hook); } diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 91e4ae8..b9b8fcb 100644 *** a/src/bin/psql/variables.c --- b/src/bin/psql/variables.c *************** CreateVariableSpace(void) *** 52,57 **** --- 52,58 ---- ptr =3D pg_malloc(sizeof *ptr); ptr->name =3D NULL; ptr->value =3D NULL; + ptr->substitute_hook =3D NULL; ptr->assign_hook =3D NULL; ptr->next =3D NULL; =20=20 *************** ParseVariableBool(const char *value, con *** 101,111 **** size_t len; bool valid =3D true; =20=20 if (value =3D=3D NULL) ! { ! *result =3D false; /* not set -> assume "off" */ ! return valid; ! } =20=20 len =3D strlen(value); =20=20 --- 102,110 ---- size_t len; bool valid =3D true; =20=20 + /* Treat "unset" as an empty string, which will lead to error below */ if (value =3D=3D NULL) ! value =3D ""; =20=20 len =3D strlen(value); =20=20 *************** ParseVariableNum(const char *value, cons *** 152,159 **** char *end; long numval; =20=20 if (value =3D=3D NULL) ! return false; errno =3D 0; numval =3D strtol(value, &end, 0); if (errno =3D=3D 0 && *end =3D=3D '\0' && end !=3D value && numval =3D= =3D (int) numval) --- 151,160 ---- char *end; long numval; =20=20 + /* Treat "unset" as an empty string, which will lead to error below */ if (value =3D=3D NULL) ! value =3D ""; !=20 errno =3D 0; numval =3D strtol(value, &end, 0); if (errno =3D=3D 0 && *end =3D=3D '\0' && end !=3D value && numval =3D= =3D (int) numval) *************** SetVariable(VariableSpace space, const c *** 235,247 **** =20=20 if (!valid_variable_name(name)) { psql_error("invalid variable name: \"%s\"\n", name); return false; } =20=20 - if (!value) - return DeleteVariable(space, name); -=20 for (previous =3D space, current =3D space->next; current; previous =3D current, current =3D current->next) --- 236,248 ---- =20=20 if (!valid_variable_name(name)) { + /* Deletion of non-existent variable is not an error */ + if (!value) + return true; psql_error("invalid variable name: \"%s\"\n", name); return false; } =20=20 for (previous =3D space, current =3D space->next; current; previous =3D current, current =3D current->next) *************** SetVariable(VariableSpace space, const c *** 249,262 **** if (strcmp(current->name, name) =3D=3D 0) { /* ! * Found entry, so update, unless hook returns false. The hook ! * may need the passed value to have the same lifespan as the ! * variable, so allocate it right away, even though we'll have to ! * free it again if the hook returns false. */ ! char *new_value =3D pg_strdup(value); bool confirmed; =20=20 if (current->assign_hook) confirmed =3D (*current->assign_hook) (new_value); else --- 250,269 ---- if (strcmp(current->name, name) =3D=3D 0) { /* ! * Found entry, so update, unless assign hook returns false. ! * ! * We must duplicate the passed value to start with. This ! * simplifies the API for substitute hooks. Moreover, some assign ! * hooks assume that the passed value has the same lifespan as the ! * variable. Having to free the string again on failure is a ! * small price to pay for keeping these APIs simple. */ ! char *new_value =3D value ? pg_strdup(value) : NULL; bool confirmed; =20=20 + if (current->substitute_hook) + new_value =3D (*current->substitute_hook) (new_value); +=20 if (current->assign_hook) confirmed =3D (*current->assign_hook) (new_value); else *************** SetVariable(VariableSpace space, const c *** 267,305 **** if (current->value) pg_free(current->value); current->value =3D new_value; } ! else pg_free(new_value); /* current->value is left unchanged */ =20=20 return confirmed; } } =20=20 ! /* not present, make new entry */ ! current =3D pg_malloc(sizeof *current); ! current->name =3D pg_strdup(name); ! current->value =3D pg_strdup(value); ! current->assign_hook =3D NULL; ! current->next =3D NULL; ! previous->next =3D current; return true; } =20=20 /* ! * Attach an assign hook function to the named variable. * ! * If the variable doesn't already exist, create it with value NULL, ! * just so we have a place to store the hook function. (Externally, ! * this isn't different from it not being defined.) * ! * The hook is immediately called on the variable's current value. This = is ! * meant to let it update any derived psql state. If the hook doesn't li= ke ! * the current value, it will print a message to that effect, but we'll i= gnore ! * it. Generally we do not expect any such failure here, because this sh= ould ! * get called before any user-supplied value is assigned. */ void ! SetVariableAssignHook(VariableSpace space, const char *name, VariableAssi= gnHook hook) { struct _variable *current, *previous; --- 274,334 ---- if (current->value) pg_free(current->value); current->value =3D new_value; +=20 + /* + * If we deleted the value, and there are no hooks to + * remember, we can discard the variable altogether. + */ + if (new_value =3D=3D NULL && + current->substitute_hook =3D=3D NULL && + current->assign_hook =3D=3D NULL) + { + previous->next =3D current->next; + free(current->name); + free(current); + } } ! else if (new_value) pg_free(new_value); /* current->value is left unchanged */ =20=20 return confirmed; } } =20=20 ! /* not present, make new entry ... unless we were asked to delete */ ! if (value) ! { ! current =3D pg_malloc(sizeof *current); ! current->name =3D pg_strdup(name); ! current->value =3D pg_strdup(value); ! current->substitute_hook =3D NULL; ! current->assign_hook =3D NULL; ! current->next =3D NULL; ! previous->next =3D current; ! } return true; } =20=20 /* ! * Attach substitute and/or assign hook functions to the named variable. ! * If you need only one hook, pass NULL for the other. * ! * If the variable doesn't already exist, create it with value NULL, just= so ! * we have a place to store the hook function(s). (The substitute hook m= ight ! * immediately change the NULL to something else; if not, this state is ! * externally the same as the variable not being defined.) * ! * The substitute hook, if given, is immediately called on the variable's ! * value. Then the assign hook, if given, is called on the variable's va= lue. ! * This is meant to let it update any derived psql state. If the assign = hook ! * doesn't like the current value, it will print a message to that effect, ! * but we'll ignore it. Generally we do not expect any such failure here, ! * because this should get called before any user-supplied value is assig= ned. */ void ! SetVariableHooks(VariableSpace space, const char *name, ! VariableSubstituteHook shook, ! VariableAssignHook ahook) { struct _variable *current, *previous; *************** SetVariableAssignHook(VariableSpace spac *** 317,324 **** if (strcmp(current->name, name) =3D=3D 0) { /* found entry, so update */ ! current->assign_hook =3D hook; ! (void) (*hook) (current->value); return; } } --- 346,357 ---- if (strcmp(current->name, name) =3D=3D 0) { /* found entry, so update */ ! current->substitute_hook =3D shook; ! current->assign_hook =3D ahook; ! if (shook) ! current->value =3D (*shook) (current->value); ! if (ahook) ! (void) (*ahook) (current->value); return; } } *************** SetVariableAssignHook(VariableSpace spac *** 327,336 **** current =3D pg_malloc(sizeof *current); current->name =3D pg_strdup(name); current->value =3D NULL; ! current->assign_hook =3D hook; current->next =3D NULL; previous->next =3D current; ! (void) (*hook) (NULL); } =20=20 /* --- 360,373 ---- current =3D pg_malloc(sizeof *current); current->name =3D pg_strdup(name); current->value =3D NULL; ! current->substitute_hook =3D shook; ! current->assign_hook =3D ahook; current->next =3D NULL; previous->next =3D current; ! if (shook) ! current->value =3D (*shook) (current->value); ! if (ahook) ! (void) (*ahook) (current->value); } =20=20 /* *************** SetVariableBool(VariableSpace space, con *** 351,392 **** bool DeleteVariable(VariableSpace space, const char *name) { ! struct _variable *current, ! *previous; !=20 ! if (!space) ! return true; !=20 ! for (previous =3D space, current =3D space->next; ! current; ! previous =3D current, current =3D current->next) ! { ! if (strcmp(current->name, name) =3D=3D 0) ! { ! if (current->assign_hook) ! { ! /* Allow deletion only if hook is okay with NULL value */ ! if (!(*current->assign_hook) (NULL)) ! return false; /* message printed by hook */ ! if (current->value) ! free(current->value); ! current->value =3D NULL; ! /* Don't delete entry, or we'd forget the hook function */ ! } ! else ! { ! /* We can delete the entry as well as its value */ ! if (current->value) ! free(current->value); ! previous->next =3D current->next; ! free(current->name); ! free(current); ! } ! return true; ! } ! } !=20 ! return true; } =20=20 /* --- 388,394 ---- bool DeleteVariable(VariableSpace space, const char *name) { ! return SetVariable(space, name, NULL); } =20=20 /* diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h index 274b4af..84be780 100644 *** a/src/bin/psql/variables.h --- b/src/bin/psql/variables.h *************** *** 18,29 **** * prevent invalid values from being assigned, and can update internal C * variables to keep them in sync with the variable's current value. * ! * A hook function is called before any attempted assignment, with the * proposed new value of the variable (or with NULL, if an \unset is being * attempted). If it returns false, the assignment doesn't occur --- it * should print an error message with psql_error() to tell the user why. * ! * When a hook function is installed with SetVariableAssignHook(), it is * called with the variable's current value (or with NULL, if it wasn't s= et * yet). But its return value is ignored in this case. The hook should = be * set before any possibly-invalid value can be assigned. --- 18,29 ---- * prevent invalid values from being assigned, and can update internal C * variables to keep them in sync with the variable's current value. * ! * An assign hook function is called before any attempted assignment, wit= h the * proposed new value of the variable (or with NULL, if an \unset is being * attempted). If it returns false, the assignment doesn't occur --- it * should print an error message with psql_error() to tell the user why. * ! * When an assign hook function is installed with SetVariableHooks(), it = is * called with the variable's current value (or with NULL, if it wasn't s= et * yet). But its return value is ignored in this case. The hook should = be * set before any possibly-invalid value can be assigned. *************** *** 31,45 **** typedef bool (*VariableAssignHook) (const char *newval); =20=20 /* * Data structure representing one variable. * * Note: if value =3D=3D NULL then the variable is logically unset, but w= e are ! * keeping the struct around so as not to forget about its hook function. */ struct _variable { char *name; char *value; VariableAssignHook assign_hook; struct _variable *next; }; --- 31,69 ---- typedef bool (*VariableAssignHook) (const char *newval); =20=20 /* + * Variables can also be given "substitute hook" functions. The substitu= te + * hook can replace values (including NULL) with other values, allowing + * normalization of variable contents. For example, for a boolean variab= le, + * we wish to interpret "\unset FOO" as "\set FOO off", and we can do that + * by installing a substitute hook. (We can use the same substitute hook + * for all bool or nearly-bool variables, which is why this responsibility + * isn't part of the assign hook.) + * + * The substitute hook is called before any attempted assignment, and bef= ore + * the assign hook if any, passing the proposed new value of the variable= as a + * malloc'd string (or NULL, if an \unset is being attempted). It can re= turn + * the same value, or a different malloc'd string, or modify the string + * in-place. It should free the passed-in value if it's not returning it. + * The substitute hook generally should not complain about erroneous valu= es; + * that's a job for the assign hook. + * + * When a substitute hook is installed with SetVariableHooks(), it is app= lied + * to the variable's current value (typically NULL, if it wasn't set yet). + * That also happens before applying the assign hook. + */ + typedef char *(*VariableSubstituteHook) (char *newval); +=20 + /* * Data structure representing one variable. * * Note: if value =3D=3D NULL then the variable is logically unset, but w= e are ! * keeping the struct around so as not to forget about its hook function(= s). */ struct _variable { char *name; char *value; + VariableSubstituteHook substitute_hook; VariableAssignHook assign_hook; struct _variable *next; }; *************** int GetVariableNum(VariableSpace space, *** 65,74 **** void PrintVariables(VariableSpace space); =20=20 bool SetVariable(VariableSpace space, const char *name, const char *valu= e); - void SetVariableAssignHook(VariableSpace space, const char *name, Variab= leAssignHook hook); bool SetVariableBool(VariableSpace space, const char *name); bool DeleteVariable(VariableSpace space, const char *name); =20=20 void PsqlVarEnumError(const char *name, const char *value, const char *s= uggestions); =20=20 #endif /* VARIABLES_H */ --- 89,101 ---- void PrintVariables(VariableSpace space); =20=20 bool SetVariable(VariableSpace space, const char *name, const char *valu= e); bool SetVariableBool(VariableSpace space, const char *name); bool DeleteVariable(VariableSpace space, const char *name); =20=20 + void SetVariableHooks(VariableSpace space, const char *name, + VariableSubstituteHook shook, + VariableAssignHook ahook); +=20 void PsqlVarEnumError(const char *name, const char *value, const char *s= uggestions); =20=20 #endif /* VARIABLES_H */ diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected= /psql.out index 420825a..026a4f0 100644 *** a/src/test/regress/expected/psql.out --- b/src/test/regress/expected/psql.out *************** invalid variable name: "invalid/name" *** 11,16 **** --- 11,33 ---- unrecognized value "foo" for "AUTOCOMMIT": boolean expected \set FETCH_COUNT foo invalid value "foo" for "FETCH_COUNT": integer expected + -- check handling of built-in boolean variable + \echo :ON_ERROR_ROLLBACK + off + \set ON_ERROR_ROLLBACK + \echo :ON_ERROR_ROLLBACK + on + \set ON_ERROR_ROLLBACK foo + unrecognized value "foo" for "ON_ERROR_ROLLBACK" + Available values are: on, off, interactive. + \echo :ON_ERROR_ROLLBACK + on + \set ON_ERROR_ROLLBACK on + \echo :ON_ERROR_ROLLBACK + on + \unset ON_ERROR_ROLLBACK + \echo :ON_ERROR_ROLLBACK + off -- \gset select 10 as test01, 20 as test02, 'Hello' as test03 \gset pref01_ \echo :pref01_test01 :pref01_test02 :pref01_test03 diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 79624b9..d823d11 100644 *** a/src/test/regress/sql/psql.sql --- b/src/test/regress/sql/psql.sql *************** *** 10,15 **** --- 10,25 ---- -- fail: invalid value for special variable \set AUTOCOMMIT foo \set FETCH_COUNT foo + -- check handling of built-in boolean variable + \echo :ON_ERROR_ROLLBACK + \set ON_ERROR_ROLLBACK + \echo :ON_ERROR_ROLLBACK + \set ON_ERROR_ROLLBACK foo + \echo :ON_ERROR_ROLLBACK + \set ON_ERROR_ROLLBACK on + \echo :ON_ERROR_ROLLBACK + \unset ON_ERROR_ROLLBACK + \echo :ON_ERROR_ROLLBACK =20=20 -- \gset =20=20 ------- =_aaaaaaaaaa0 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers ------- =_aaaaaaaaaa0--