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

Reply via email to