I wrote: > This would allow the hook to distinguish between initialization and > unsetting, which in turn will allow it to deny the \unset in the > cases when it doesn't make any sense conceptually (like AUTOCOMMIT).
I notice that in the commited patch, you added the ability for DeleteVariable() to reject the deletion if the hook disagrees. + { + /* Allow deletion only if hook is okay with NULL value */ + if (!(*current->assign_hook) (NULL)) + return false; /* message printed by hook */ But this can't happen in practice because as mentioned just upthread the hook called with NULL doesn't know if the variable is getting unset or initialized, so rejecting on NULL is not an option. Attached is a proposed patch to add initial values to SetVariableAssignHook() to solve this problem, and an example for \unset AUTOCOMMIT being denied by the hook. As a side effect, we see all buit-in variables when issuing \set rather than just a few. Does it make sense? Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 0574b5b..631b3f6 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -166,14 +166,6 @@ main(int argc, char *argv[]) SetVariable(pset.vars, "VERSION", PG_VERSION_STR); - /* 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); - parse_psql_options(argc, argv, &options); /* @@ -785,6 +777,11 @@ showVersion(void) static bool autocommit_hook(const char *newval) { + if (newval == NULL) + { + psql_error("built-in variable %s cannot be unset.\n", "AUTOCOMMIT"); + return false; + } return ParseVariableBool(newval, "AUTOCOMMIT", &pset.autocommit); } @@ -1002,20 +999,20 @@ EstablishVariableSpace(void) { pset.vars = CreateVariableSpace(); - 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); + SetVariableAssignHook(pset.vars, "AUTOCOMMIT", autocommit_hook, "on"); + SetVariableAssignHook(pset.vars, "ON_ERROR_STOP", on_error_stop_hook, "off"); + SetVariableAssignHook(pset.vars, "QUIET", quiet_hook, "off"); + SetVariableAssignHook(pset.vars, "SINGLELINE", singleline_hook, "off"); + SetVariableAssignHook(pset.vars, "SINGLESTEP", singlestep_hook, "off"); + SetVariableAssignHook(pset.vars, "FETCH_COUNT", fetch_count_hook, "-1"); + SetVariableAssignHook(pset.vars, "ECHO", echo_hook, "none"); + SetVariableAssignHook(pset.vars, "ECHO_HIDDEN", echo_hidden_hook, "off"); + SetVariableAssignHook(pset.vars, "ON_ERROR_ROLLBACK", on_error_rollback_hook, "off"); + SetVariableAssignHook(pset.vars, "COMP_KEYWORD_CASE", comp_keyword_case_hook, "preserve-upper"); + SetVariableAssignHook(pset.vars, "HISTCONTROL", histcontrol_hook, "none"); + SetVariableAssignHook(pset.vars, "PROMPT1", prompt1_hook, DEFAULT_PROMPT1); + SetVariableAssignHook(pset.vars, "PROMPT2", prompt2_hook, DEFAULT_PROMPT2); + SetVariableAssignHook(pset.vars, "PROMPT3", prompt3_hook, DEFAULT_PROMPT3); + SetVariableAssignHook(pset.vars, "VERBOSITY", verbosity_hook, "default"); + SetVariableAssignHook(pset.vars, "SHOW_CONTEXT", show_context_hook, "errors"); } diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c index 91e4ae8..2e5c3e0 100644 --- a/src/bin/psql/variables.c +++ b/src/bin/psql/variables.c @@ -288,9 +288,8 @@ SetVariable(VariableSpace space, const char *name, const char *value) /* * 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.) + * If the variable doesn't already exist, create it with init_value as the + * content. init_value must not be NULL. * * 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 like @@ -299,7 +298,10 @@ SetVariable(VariableSpace space, const char *name, const char *value) * get called before any user-supplied value is assigned. */ void -SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook) +SetVariableAssignHook(VariableSpace space, + const char *name, + VariableAssignHook hook, + const char *init_value) { struct _variable *current, *previous; @@ -326,11 +328,11 @@ SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook /* not present, make new entry */ current = pg_malloc(sizeof *current); current->name = pg_strdup(name); - current->value = NULL; + current->value = pg_strdup(init_value); current->assign_hook = hook; current->next = NULL; previous->next = current; - (void) (*hook) (NULL); + (void) (*hook) (current->value); } /* diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h index 274b4af..94e7fb8 100644 --- a/src/bin/psql/variables.h +++ b/src/bin/psql/variables.h @@ -24,9 +24,9 @@ * 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 set - * yet). But its return value is ignored in this case. The hook should be - * set before any possibly-invalid value can be assigned. + * called with the variable's initial value, but its return value is ignored + * in this case. + * The hook should be set before any possibly-invalid value can be assigned. */ typedef bool (*VariableAssignHook) (const char *newval); @@ -65,7 +65,10 @@ int GetVariableNum(VariableSpace space, void PrintVariables(VariableSpace space); bool SetVariable(VariableSpace space, const char *name, const char *value); -void SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook); +void SetVariableAssignHook(VariableSpace space, + const char *name, + VariableAssignHook hook, + const char *init_value); bool SetVariableBool(VariableSpace space, const char *name); bool DeleteVariable(VariableSpace space, const char *name);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers