Обсуждение: Re: [HACKERS] Improvements in psql hooks for variables
"Daniel Verite" <daniel@manitou-mail.org> writes:
> [ psql-var-hooks-v6.patch ]
I took a quick look through this. It seems to be going in generally
the right direction, but here's a couple of thoughts:
* It seems like you're making life hard for yourself, and confusing for
readers, by having opposite API conventions at different levels. You've
got ParseVariableBool returning the parsed value as function result with
validity flag going into an output parameter, but the boolean variable
hooks do it the other way.
I'm inclined to think that the best choice is for the function result
to be the ok/not ok flag, and pass the variable-to-be-modified as an
output parameter. That fits better with the notion that the variable
is not to be modified on failure. You're having to change every caller
of ParseVariableBool anyway, so changing them a bit more doesn't seem
like a problem. I think actually you don't need generic_boolean_hook()
at all if you do that; it appears to do nothing except fix this impedance
mismatch.
Also, why aren't you using ParseVariableBool's existing ability to report
errors? It seems like this:
else if (value)
! {
! bool valid;
! bool newval = ParseVariableBool(value, NULL, &valid);
! if (valid)
! popt->topt.expanded = newval;
! else
! {
! psql_error("unrecognized value \"%s\" for \"%s\"\n",
! value, param);
! return false;
! }
! }
is really the hard way and you could have just done
- popt->topt.expanded = ParseVariableBool(value, param);
+ success = ParseVariableBool(value, param, &popt->topt.expanded);
I'd do it the same way for ParseCheckVariableNum. Also, is there a reason
why that's adding new code rather than changing ParseVariableNum?
I think if we're going to have a policy that bool variables must be valid
bools, there's no reason not to insist similarly for integers.
* More attention is needed to comments. Most glaringly, you've changed
the API for VariableAssignHook without any attention to the API spec
above that typedef. (That comment block is a bit confused anyway, since
half of it is an overall explanation of what the module does and the
other half is an API spec for the hooks. I think I'd move the overall
explanation into the file header comment.) Also, I don't like this way
of explaining an output parameter:
+ * "valid" points to a bool reporting whether the value was valid.
because it's not really clear that the function is setting that value
rather than expecting it to be set to something by the caller.
Assuming you take my advice in the previous point, you could document
ParseVariableBool and ParseVariableNum along the lines of
* Returns true if string contents represent a valid value, false if not.* If the string is valid, the value is stored
into*value, else *value* remains unchanged.
regards, tom lane
Tom Lane wrote:
> "Daniel Verite" <daniel@manitou-mail.org> writes:
> > [ psql-var-hooks-v6.patch ]
>
> I took a quick look through this. It seems to be going in generally
> the right direction, but here's a couple of thoughts:
Thanks for looking into this!
> I'm inclined to think that the best choice is for the function result
> to be the ok/not ok flag, and pass the variable-to-be-modified as an
> output parameter. That fits better with the notion that the variable
> is not to be modified on failure.
Agreed, if never clobbering the variable, having the valid/invalid state
returned by ParseVariableBool() allows for simpler code. I'm changing it
that way.
> Also, why aren't you using ParseVariableBool's existing ability to report
> errors?
Its' because there are two cases:
- either only a boolean can be given to the command or variable,
in which we let ParseVariableBool() tell: unrecognized value "bogus" for "command": boolean expected
- or other parameters besides boolean are acceptable, in which case we
can't say "boolean expected", because in fact a boolean is no more or
less expected than the other valid values.
We could shave code by reducing ParseVariableBool()'s error message to: unrecognized value "bogus" for "name"
clearing the distinction between [only booleans are expected]
and [booleans or enum are expected].
Then almost all callers that have their own message could rely
on ParseVariableBool() instead, as they did previously.
Do we care about the "boolean expected" part of the error message?
> else if (value)
> ! {
> ! bool valid;
> ! bool newval = ParseVariableBool(value, NULL, &valid);
> ! if (valid)
> ! popt->topt.expanded = newval;
> ! else
> ! {
> ! psql_error("unrecognized value \"%s\" for \"%s\"\n",
> ! value, param);
> ! return false;
> ! }
> ! }
> is really the hard way and you could have just done
>
> - popt->topt.expanded = ParseVariableBool(value, param);
> + success = ParseVariableBool(value, param,
> &popt->topt.expanded);
I get the idea, except that in this example, the compiler is
unhappy because popt->topt.expanded is not a bool, and an
explicit cast here would be kludgy.
For the error printing part, it would go away with the above
suggestion
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
Tom Lane wrote: > I took a quick look through this. It seems to be going in generally > the right direction, but here's a couple of thoughts: Here's an update with these changes: per Tom's suggestions upthread: - change ParseVariableBool() signature to return validity as bool. - remove ParseCheckVariableNum() in favor of using tightened up ParseVariableNum() and GetVariableNum(). - updated header comments in variables.h other changes: - autocommit_hook rejects transitions from OFF to ON when inside a transaction, per suggestion of Rahila Syed (which was the original motivation for the set of changes of this patch). - slight doc update for HISTCONTROL (values outside of enum not longer allowed) - add enum-style suggestions on invalid input for \pset x, \pset pager, and \set of ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE, HISTCONTROL, VERBOSITY, SHOW_CONTEXT, \x, \pager Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
I just wrote: > - add enum-style suggestions on invalid input for \pset x, \pset pager, > and \set of ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE, > HISTCONTROL, VERBOSITY, SHOW_CONTEXT, \x, \pager There's no such thing as \pager, I meant to write: \pset x, \pset pager, and \set of ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE, HISTCONTROL, VERBOSITY, SHOW_CONTEXT Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Here's an update with these changes:
I scanned this patch very quickly and think it addresses my previous
stylistic objections. Rahila is the reviewer of record though, so
I'll wait for his comments before going further.
regards, tom lane
Hello,
The patch works fine on applying on latest master branch and testing it for various variables as listed in PsqlSettings struct.On Wed, Jan 25, 2017 at 1:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Here's an update with these changes:
I scanned this patch very quickly and think it addresses my previous
stylistic objections. Rahila is the reviewer of record though, so
I'll wait for his comments before going further.
regards, tom lane
Hello,
Please consider following comments on the patch.
Please consider following comments on the patch.
In function ParseVariableNum,
> if (!val || !val[0])
> return false;
Check for 'val == NULL' as in above condition is done even in callers of ParseVariableNum().
There should be only one such check.
>+ psql_error("Invalid value \"%s\" for \"%s\"\nAn integer is expected\n",
Cant we have this error in ParseVariableNum() similar to ParseVariableBool() ?
>+ /*
>+ * 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);
>+ }
The above change in autocommit behaviour needs to be documented.
Thank you,
Rahila Syed
On Wed, Jan 25, 2017 at 5:48 PM, Rahila Syed <rahilasyed90@gmail.com> wrote:
Rahila SyedThank you,I will post further comments on patch soon.Hello,The patch works fine on applying on latest master branch and testing it for various variables as listed in PsqlSettings struct.On Wed, Jan 25, 2017 at 1:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:"Daniel Verite" <daniel@manitou-mail.org> writes:
> Here's an update with these changes:
I scanned this patch very quickly and think it addresses my previous
stylistic objections. Rahila is the reviewer of record though, so
I'll wait for his comments before going further.
regards, tom lane
Rahila Syed <rahilasyed90@gmail.com> writes:
> Please consider following comments on the patch.
> In function ParseVariableNum,
>> if (!val || !val[0])
>> return false;
> Check for 'val == NULL' as in above condition is done even in callers of
> ParseVariableNum().
> There should be only one such check.
Meh ... I don't think it's unreasonable for ParseVariableNum to defend
itself against that. The callers might or might not be applying a check
--- for them, it would be a matter of do they need to detect presence
or absence of an option, but not about whether the option value is valid.
>> + psql_error("Invalid value \"%s\" for \"%s\"\nAn integer is
>> expected\n",
> Cant we have this error in ParseVariableNum() similar to
> ParseVariableBool() ?
Agreed, error messages should be stylistically similar. I'm not sure they
can be exactly alike though. Right now the patch has ParseVariableBool
saying
+ psql_error("unrecognized value \"%s\" for \"%s\": boolean expected\n",
+ value, name);
while callers that don't use that error use PsqlVarEnumError which has
+ psql_error("Unrecognized value \"%s\" for \"%s\"\nAvailable values: %s\n",
+ value, name, suggestions);
and then ParseVariableNum is as above. So first off we've got a
capitalization inconsistency. Project style for backend messages is
no initial cap; psql seems not to be on board with that entirely,
but I'm definitely inclined to go with it here. As for "invalid"
vs. "unrecognized", I'm not sure "unrecognized" really fits the
bill for "this isn't an integer". So I'm inclined to leave that
alone. I suggest we go with these:
"invalid value \"%s\" for \"%s\": integer expected\n"
"unrecognized value \"%s\" for \"%s\": boolean expected\n"
"unrecognized value \"%s\" for \"%s\"\nAvailable values are: %s\n."
where the last case is following the message style for hints.
>> + * Switching AUTOCOMMIT from OFF to ON is not allowed when inside a
>> + * transaction, because it cannot be effective until the current
>> + * transaction is ended.
> The above change in autocommit behaviour needs to be documented.
Yeah, definitely.
I'll go ahead and push the patch with these fixes. Thanks for reviewing!
regards, tom lane
BTW, while testing this patch I noticed that the error reports are
a tad redundant:
regression=# \set AUTOCOMMIT foo
unrecognized value "foo" for "AUTOCOMMIT": boolean expected
\set: error while setting variable
regression=#
The "error while setting variable" message seems entirely content-free.
I think we should drop that and instead establish a rule that SetVariable
should print a message for itself about any failure. (There are a lot
of call sites that don't check for or print a message, but that's only
because they aren't expecting failure. If one were to happen, printing
a message doesn't seem unreasonable.) That would in turn propagate into
an API requirement that var hooks that return FALSE are responsible for
printing a message about the reason, which is why it would be appropriate
to make that change as part of this patch.
Barring objections PDQ, I'll make this change.
regards, tom lane
I wrote:
> Rahila Syed <rahilasyed90@gmail.com> writes:
>>> + * Switching AUTOCOMMIT from OFF to ON is not allowed when inside a
>>> + * transaction, because it cannot be effective until the current
>>> + * transaction is ended.
>> The above change in autocommit behaviour needs to be documented.
> Yeah, definitely.
Actually ... while trying to write some documentation for that, I found
myself wondering why we need such a prohibition at all. If you are inside
a transaction, then autocommit has no effect until after you get out of
the transaction, and the documentation about it seems clear enough on the
point. Also, if you want to argue that allowing it to change intra-
transaction is too confusing, why would we only forbid this direction
of change and not both directions?
I'm afraid we might be breaking some peoples' scripts to no particularly
good end, so I'm going to leave this out of the committed patch. If you
think this really is a valid change to make, we can commit it separately,
but let's discuss it on its own merits.
regards, tom lane
So I pushed this, and the buildfarm members that are testing RedisFDW
immediately fell over:
*** /home/andrew/bf/root/HEAD/redis_fdw.build/test/expected/redis_fdw.out 2017-01-30 18:20:27.440677318 -0500
--- /home/andrew/bf/root/HEAD/redis_fdw.build/test/results/redis_fdw.out 2017-01-30 18:32:33.404677320 -0500
***************
*** 26,31 ****
--- 26,32 ---- options (database '15', tabletype 'zset'); -- make sure they are all empty - if any are not stop
thescript right now \set ON_ERROR_STOP
+ unrecognized value "" for "ON_ERROR_STOP": boolean expected do $$ declare rows bigint;
======================================================================
Evidently, this test script is relying on the preceding behavior that
setting a bool variable to an empty string was equivalent to setting
it to "true". If it's just that script I would be okay with saying
"well, it's a bug in that script" ... but I'm a bit worried that this
may be the tip of the iceberg, ie maybe a lot of people have done
things like this. Should we reconsider the decision to reject empty
strings in ParseVariableBool?
regards, tom lane
Tom Lane wrote: > Also, if you want to argue that allowing it to change intra- > transaction is too confusing, why would we only forbid this direction > of change and not both directions? The thread "Surprising behaviour of \set AUTOCOMMIT ON" at: https://www.postgresql.org/message-id/CAH2L28sTP-9dio3X1AaZRyWb0-ANAx6BDBi37TGmvw1hBiu0oA%40mail.gmail.com seemed to converge towards the conclusion implemented in the autocommit_hook proposed in the patch. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
Tom Lane wrote: > Evidently, this test script is relying on the preceding behavior that > setting a bool variable to an empty string was equivalent to setting > it to "true". If it's just that script I would be okay with saying > "well, it's a bug in that script" ... but I'm a bit worried that this > may be the tip of the iceberg, ie maybe a lot of people have done > things like this. Should we reconsider the decision to reject empty > strings in ParseVariableBool? Sigh. It was considered upthread, I'm requoting the relevant bit: <quote> if (pg_strncasecmp(value, "true", len) == 0) return true; It happens that "" as a value yields the same resultthan "true" for this test so it passes, but it seems rather unintentional. The resulting problem from the POV of the user is that we can do that, for instance: test=> \set AUTOCOMMIT without error message or feedback, and that leaves us without much clue about autocommit being enabled: test=> \echo :AUTOCOMMIT test=> So I've changed ParseVariableBool() in v4 to reject empty string as an invalid boolean (but not NULL since the startup logicrequires NULL meaning false in the early initialization of these variables). "make check" seems OK with that, I hope it doesn't cause any regression elsewhere. </quote> So it does cause regressions. I don't know if we should reaccept empty strings immediately to fix that, but in the long run, I think that the above situation with the empty :AUTOCOMMIT is not really sustainable: when we extend what we do with variables (/if /endif and so on), it will become even more of a problem. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Tom Lane wrote:
>> Evidently, this test script is relying on the preceding behavior that
>> setting a bool variable to an empty string was equivalent to setting
>> it to "true". If it's just that script I would be okay with saying
>> "well, it's a bug in that script" ... but I'm a bit worried that this
>> may be the tip of the iceberg, ie maybe a lot of people have done
>> things like this. Should we reconsider the decision to reject empty
>> strings in ParseVariableBool?
> ...
> So it does cause regressions. I don't know if we should reaccept
> empty strings immediately to fix that, but in the long run, I think
> that the above situation with the empty :AUTOCOMMIT is not really
> sustainable: when we extend what we do with variables
> (/if /endif and so on), it will become even more of a problem.
Yeah, in a green field we'd certainly not allow this, but the problem
I'm having is that in all previous versions you could do, eg,
\set ON_ERROR_STOP
... stuff expecting ON_ERROR_STOP to be on
\unset ON_ERROR_STOP
... stuff expecting ON_ERROR_STOP to be off
and it works, and looks perfectly natural, and people may well be relying
on that. Especially since the docs aren't very clear that this shouldn't
work --- psql-ref.sgml repeatedly uses phrases like "FOO is set" to
indicate that the boolean variable FOO is considered to be "on".
Moreover, the committed patch is inconsistent in that it forbids
only one of the above. Why is it okay to treat unset as "off",
but not okay to treat the default empty-string value as "on"?
Maybe it's worth breaking backwards compatibility on this point, but
I'm feeling unconvinced. This seems rather different from rejecting
clearly-wrongly-spelled values.
One possible compromise that would address your concern about display
is to modify the hook API some more so that variable hooks could actually
substitute new values. Then for example the bool-variable hooks could
effectively replace "\set AUTOCOMMIT" by "\set AUTOCOMMIT on" and
"\unset AUTOCOMMIT" by "\set AUTOCOMMIT off", ensuring that interrogation
of their values always produces sane results.
regards, tom lane
Tom Lane wrote: > Moreover, the committed patch is inconsistent in that it forbids > only one of the above. Why is it okay to treat unset as "off", > but not okay to treat the default empty-string value as "on"? Treating unset (NULL in the value) as "off" comes from the fact that except AUTOCOMMIT, our built-in variables are not initialized with a default value. For instance we call this in EstablishVariableSpace(); SetVariableAssignHook(pset.vars, "ON_ERROR_STOP",on_error_stop_hook); then on_error_stop_hook is called with NULL as the value then ParseVariableBool(NULL, "ON_ERROR_STOP", &pset.on_error_stop) is what initializes pset.on_error_stop to false. The same happens if/when the variable is unset. Again the hook is called with NULL, and it sets back the pset.* variable in a hardcoded default state, which is false for all booleans. Incidentally I want to suggest to change that, so that all variables should be initialized with a non-NULL value right from the start, and the value would possibly come to NULL only if it's unset. 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). Forcing values for our built-in variables would also avoid the following: =# \echo :ON_ERROR_STOP :ON_ERROR_STOP Even if the variable ON_ERROR_STOP does exist in the VariableSpace and has a hook and there's an initialized corresponding pset.on_error_stop, from the user's viewpoint, it's as if the variable doesn't exist until they intialize it explicitly. I suggest that it doesn't make much sense and it would be better to have that instead right from the start: =# \echo :ON_ERROR_STOP off > One possible compromise that would address your concern about display > is to modify the hook API some more so that variable hooks could actually > substitute new values. Then for example the bool-variable hooks could > effectively replace "\set AUTOCOMMIT" by "\set AUTOCOMMIT on" and > "\unset AUTOCOMMIT" by "\set AUTOCOMMIT off", ensuring that interrogation > of their values always produces sane results. Agreed, that looks like a good compromise. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Tom Lane wrote:
>> One possible compromise that would address your concern about display
>> is to modify the hook API some more so that variable hooks could actually
>> substitute new values. Then for example the bool-variable hooks could
>> effectively replace "\set AUTOCOMMIT" by "\set AUTOCOMMIT on" and
>> "\unset AUTOCOMMIT" by "\set AUTOCOMMIT off", ensuring that interrogation
>> of their values always produces sane results.
> Agreed, that looks like a good compromise.
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.
Maybe we should merge SetVariableSubstituteHook and SetVariableAssignHook
into one function?
regards, tom lane
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 0574b5b..985cfcb 100644
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** showVersion(void)
*** 777,787 ****
/*
! * Assign hooks for psql variables.
*
* This isn't an amazingly good place for them, but neither is anywhere else.
*/
static bool
autocommit_hook(const char *newval)
{
--- 777,804 ----
/*
! * Substitute hooks and assign hooks for psql variables.
*
* This isn't an amazingly good place for them, but neither is anywhere else.
*/
+ static char *
+ bool_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ {
+ /* "\unset FOO" becomes "\set FOO off" */
+ newval = pg_strdup("off");
+ }
+ else if (newval[0] == '\0')
+ {
+ /* "\set FOO" becomes "\set FOO on" */
+ pg_free(newval);
+ newval = pg_strdup("on");
+ }
+ return newval;
+ }
+
static bool
autocommit_hook(const char *newval)
{
*************** EstablishVariableSpace(void)
*** 1002,1015 ****
--- 1019,1039 ----
{
pset.vars = CreateVariableSpace();
+ SetVariableSubstituteHook(pset.vars, "AUTOCOMMIT", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "AUTOCOMMIT", autocommit_hook);
+ SetVariableSubstituteHook(pset.vars, "ON_ERROR_STOP", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "ON_ERROR_STOP", on_error_stop_hook);
+ SetVariableSubstituteHook(pset.vars, "QUIET", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "QUIET", quiet_hook);
+ SetVariableSubstituteHook(pset.vars, "SINGLELINE", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "SINGLELINE", singleline_hook);
+ SetVariableSubstituteHook(pset.vars, "SINGLESTEP", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "SINGLESTEP", singlestep_hook);
SetVariableAssignHook(pset.vars, "FETCH_COUNT", fetch_count_hook);
SetVariableAssignHook(pset.vars, "ECHO", echo_hook);
+ SetVariableSubstituteHook(pset.vars, "ECHO_HIDDEN", bool_substitute_hook);
SetVariableAssignHook(pset.vars, "ECHO_HIDDEN", echo_hidden_hook);
+ SetVariableSubstituteHook(pset.vars, "ON_ERROR_ROLLBACK", bool_substitute_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);
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 91e4ae8..61c4ccc 100644
*** a/src/bin/psql/variables.c
--- b/src/bin/psql/variables.c
*************** CreateVariableSpace(void)
*** 52,57 ****
--- 52,58 ----
ptr = pg_malloc(sizeof *ptr);
ptr->name = NULL;
ptr->value = NULL;
+ ptr->substitute_hook = NULL;
ptr->assign_hook = NULL;
ptr->next = NULL;
*************** ParseVariableBool(const char *value, con
*** 101,111 ****
size_t len;
bool valid = true;
if (value == NULL)
! {
! *result = false; /* not set -> assume "off" */
! return valid;
! }
len = strlen(value);
--- 102,110 ----
size_t len;
bool valid = true;
+ /* Treat "unset" as an empty string, which will lead to error below */
if (value == NULL)
! value = "";
len = strlen(value);
*************** ParseVariableNum(const char *value, cons
*** 152,159 ****
char *end;
long numval;
if (value == NULL)
! return false;
errno = 0;
numval = strtol(value, &end, 0);
if (errno == 0 && *end == '\0' && end != value && numval == (int) numval)
--- 151,160 ----
char *end;
long numval;
+ /* Treat "unset" as an empty string, which will lead to error below */
if (value == NULL)
! value = "";
!
errno = 0;
numval = strtol(value, &end, 0);
if (errno == 0 && *end == '\0' && end != value && numval == (int) numval)
*************** SetVariable(VariableSpace space, const c
*** 235,247 ****
if (!valid_variable_name(name))
{
psql_error("invalid variable name: \"%s\"\n", name);
return false;
}
- if (!value)
- return DeleteVariable(space, name);
-
for (previous = space, current = space->next;
current;
previous = current, current = current->next)
--- 236,248 ----
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;
}
for (previous = space, current = space->next;
current;
previous = current, current = current->next)
*************** SetVariable(VariableSpace space, const c
*** 249,262 ****
if (strcmp(current->name, name) == 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 = pg_strdup(value);
bool confirmed;
if (current->assign_hook)
confirmed = (*current->assign_hook) (new_value);
else
--- 250,269 ----
if (strcmp(current->name, name) == 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 = value ? pg_strdup(value) : NULL;
bool confirmed;
+ if (current->substitute_hook)
+ new_value = (*current->substitute_hook) (new_value);
+
if (current->assign_hook)
confirmed = (*current->assign_hook) (new_value);
else
*************** SetVariable(VariableSpace space, const c
*** 267,288 ****
if (current->value)
pg_free(current->value);
current->value = new_value;
}
! else
pg_free(new_value); /* current->value is left unchanged */
return confirmed;
}
}
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
! current->value = pg_strdup(value);
current->assign_hook = NULL;
current->next = NULL;
previous->next = current;
! return true;
}
/*
--- 274,358 ----
if (current->value)
pg_free(current->value);
current->value = new_value;
+
+ /*
+ * If we deleted the value, and there are no hooks to
+ * remember, we can discard the variable altogether.
+ */
+ if (new_value == NULL &&
+ current->substitute_hook == NULL &&
+ current->assign_hook == NULL)
+ {
+ previous->next = current->next;
+ free(current->name);
+ free(current);
+ }
}
! else if (new_value)
pg_free(new_value); /* current->value is left unchanged */
return confirmed;
}
}
+ /* not present, make new entry ... unless we were asked to delete */
+ if (value)
+ {
+ current = pg_malloc(sizeof *current);
+ current->name = pg_strdup(name);
+ current->value = pg_strdup(value);
+ current->substitute_hook = NULL;
+ current->assign_hook = NULL;
+ current->next = NULL;
+ previous->next = current;
+ }
+ return true;
+ }
+
+ /*
+ * Attach a substitute 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. (But the hook
+ * might immediately change the NULL to something else.)
+ *
+ * The hook is immediately called on the variable's current value.
+ */
+ void
+ SetVariableSubstituteHook(VariableSpace space, const char *name,
+ VariableSubstituteHook hook)
+ {
+ struct _variable *current,
+ *previous;
+
+ if (!space || !name)
+ return;
+
+ if (!valid_variable_name(name))
+ return;
+
+ for (previous = space, current = space->next;
+ current;
+ previous = current, current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* found entry, so update */
+ current->substitute_hook = hook;
+ current->value = (*hook) (current->value);
+ return;
+ }
+ }
+
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
! current->value = NULL;
! current->substitute_hook = hook;
current->assign_hook = NULL;
current->next = NULL;
previous->next = current;
! current->value = (*hook) (current->value);
}
/*
*************** SetVariable(VariableSpace space, const c
*** 299,305 ****
* get called before any user-supplied value is assigned.
*/
void
! SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook)
{
struct _variable *current,
*previous;
--- 369,376 ----
* get called before any user-supplied value is assigned.
*/
void
! SetVariableAssignHook(VariableSpace space, const char *name,
! VariableAssignHook hook)
{
struct _variable *current,
*previous;
*************** SetVariableAssignHook(VariableSpace spac
*** 327,332 ****
--- 398,404 ----
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
current->value = NULL;
+ current->substitute_hook = NULL;
current->assign_hook = hook;
current->next = NULL;
previous->next = current;
*************** SetVariableBool(VariableSpace space, con
*** 351,392 ****
bool
DeleteVariable(VariableSpace space, const char *name)
{
! struct _variable *current,
! *previous;
!
! if (!space)
! return true;
!
! for (previous = space, current = space->next;
! current;
! previous = current, current = current->next)
! {
! if (strcmp(current->name, name) == 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 = 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 = current->next;
! free(current->name);
! free(current);
! }
! return true;
! }
! }
!
! return true;
}
/*
--- 423,429 ----
bool
DeleteVariable(VariableSpace space, const char *name)
{
! return SetVariable(space, name, NULL);
}
/*
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index 274b4af..f382bfd 100644
*** a/src/bin/psql/variables.h
--- b/src/bin/psql/variables.h
***************
*** 18,45 ****
* 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 set
* yet). 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);
/*
* Data structure representing one variable.
*
* 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.
*/
struct _variable
{
char *name;
char *value;
VariableAssignHook assign_hook;
struct _variable *next;
};
--- 18,70 ----
* 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, 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 an assign 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.
*/
typedef bool (*VariableAssignHook) (const char *newval);
/*
+ * Variables can also be given "substitute hook" functions. The substitute
+ * hook can replace values (including NULL) with other values, allowing
+ * normalization of variable contents. For example, for a boolean variable,
+ * 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 before
+ * 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 return
+ * 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 values;
+ * that's a job for the assign hook.
+ *
+ * When a substitute hook is installed with SetVariableSubstituteHook(),
+ * it is applied to the variable's current value (typically NULL, if it wasn't
+ * set yet). It's usually best to do that before installing the assign hook
+ * if there is one.
+ */
+ typedef char *(*VariableSubstituteHook) (char *newval);
+
+ /*
* Data structure representing one variable.
*
* 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(s).
*/
struct _variable
{
char *name;
char *value;
+ VariableSubstituteHook substitute_hook;
VariableAssignHook assign_hook;
struct _variable *next;
};
*************** int GetVariableNum(VariableSpace space,
*** 65,70 ****
--- 90,96 ----
void PrintVariables(VariableSpace space);
bool SetVariable(VariableSpace space, const char *name, const char *value);
+ void SetVariableSubstituteHook(VariableSpace space, const char *name, VariableSubstituteHook hook);
void SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
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
-- \gset
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
"Daniel Verite" <daniel@manitou-mail.org> writes:
> I notice that in the commited patch, you added the ability
> for DeleteVariable() to reject the deletion if the hook
> disagrees.
Right.
> 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.
It would have required the caller to set a value before installing the
hook, which would require some reshuffling of responsibility. In the
draft patch I sent a little bit ago, this is handled by installing a
"substitution hook" first, and that hook transmogrifies NULL into an
allowed setting. That gets to the same place in a slightly different
way, but it also covers allowing "\unset FOO", which inserting initial
values wouldn't.
> 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.
I think disallowing \unset is a nonstarter on compatibility grounds.
We should allow \unset but treat it like setting to "off" (or whatever
the default value is).
regards, tom lane
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
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.
</para>
</listitem>
</varlistentry>
--- 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.
</para>
</listitem>
</varlistentry>
*************** lo_import 152801
*** 2692,2698 ****
class="parameter">name</replaceable> to <replaceable
class="parameter">value</replaceable>, 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 <command>\unset</command> command.
</para>
--- 2692,2698 ----
class="parameter">name</replaceable> to <replaceable
class="parameter">value</replaceable>, 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 <command>\unset</command> command.
</para>
*************** lo_import 152801
*** 2709,2717 ****
</para>
<para>
! Although you are welcome to set any variable to anything you
! want, <application>psql</application> treats several variables
! as special. They are documented in the section about variables.
</para>
<note>
--- 2709,2719 ----
</para>
<para>
! Certain variables are special, in that they
! control <application>psql</application>'s behavior or are
! automatically set to reflect connection state. These variables are
! documented in <xref linkend="APP-PSQL-variables"
! endterm="APP-PSQL-variables-title">, below.
</para>
<note>
*************** testdb=> <userinput>\setenv LESS -imx
*** 2835,2840 ****
--- 2837,2850 ----
Unsets (deletes) the <application>psql</> variable <replaceable
class="parameter">name</replaceable>.
</para>
+
+ <para>
+ Most variables that control <application>psql</application>'s behavior
+ cannot be unset; instead, an <literal>\unset</> command is interpreted
+ as setting them to their default values.
+ See <xref linkend="APP-PSQL-variables"
+ endterm="APP-PSQL-variables-title">, below.
+ </para>
</listitem>
</varlistentry>
*************** bar
*** 3053,3059 ****
<para>
If you call <command>\set</command> without a second argument, the
! variable is set, with an empty string as value. To unset (i.e., delete)
a variable, use the command <command>\unset</command>. To show the
values of all variables, call <command>\set</command> without any argument.
</para>
--- 3063,3069 ----
<para>
If you call <command>\set</command> without a second argument, the
! variable is set to an empty-string value. To unset (i.e., delete)
a variable, use the command <command>\unset</command>. To show the
values of all variables, call <command>\set</command> without any argument.
</para>
*************** 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 specially
! treated variables follows.
</para>
<variablelist>
--- 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.
! </para>
!
! <para>
! Variables that control <application>psql</application>'s behavior
! generally cannot be unset or set to invalid values. An <literal>\unset</>
! command is allowed but is interpreted as setting the variable to its
! default value. A <literal>\set</> command without a second argument is
! interpreted as setting the variable to <literal>on</>, for control
! variables that accept that value, and is rejected for others. Also,
! control variables that accept the values <literal>on</>
! and <literal>off</> will also accept other common spellings of Boolean
! values, such as <literal>true</> and <literal>false</>.
! </para>
!
! <para>
! The specially treated variables are:
</para>
<variablelist>
*************** bar
*** 3153,3159 ****
<para>
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.
</para>
</listitem>
</varlistentry>
--- 3178,3184 ----
<para>
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.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3171,3178 ****
as it is sent to the server. The switch to select this behavior is
<option>-e</option>. If set to <literal>errors</literal>, then only
failed queries are displayed on standard error output. The switch
! for this behavior is <option>-b</option>. If unset, or if set to
! <literal>none</literal>, then no queries are displayed.
</para>
</listitem>
</varlistentry>
--- 3196,3203 ----
as it is sent to the server. The switch to select this behavior is
<option>-e</option>. If set to <literal>errors</literal>, then only
failed queries are displayed on standard error output. The switch
! for this behavior is <option>-b</option>. If set to
! <literal>none</literal> (the default), then no queries are displayed.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3187,3194 ****
<productname>PostgreSQL</productname> internals and provide
similar functionality in your own programs. (To select this behavior
on program start-up, use the switch <option>-E</option>.) If you set
! the variable to the value <literal>noexec</literal>, the queries are
just shown but are not actually sent to the server and executed.
</para>
</listitem>
</varlistentry>
--- 3212,3220 ----
<productname>PostgreSQL</productname> internals and provide
similar functionality in your own programs. (To select this behavior
on program start-up, use the switch <option>-E</option>.) If you set
! this variable to the value <literal>noexec</literal>, the queries are
just shown but are not actually sent to the server and executed.
+ The default value is <literal>off</>.
</para>
</listitem>
</varlistentry>
*************** 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 <literal>\encoding</>, but it can be unset.
</para>
</listitem>
</varlistentry>
--- 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 <literal>\encoding</>, but it can be changed or unset.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3209,3215 ****
<term><varname>FETCH_COUNT</varname></term>
<listitem>
<para>
! If this variable is set to an integer value > 0,
the results of <command>SELECT</command> 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 ----
<term><varname>FETCH_COUNT</varname></term>
<listitem>
<para>
! If this variable is set to an integer value greater than zero,
the results of <command>SELECT</command> 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.
</para>
+
+ <para>
+ <varname>FETCH_COUNT</varname> 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.
+ </para>
+
<tip>
<para>
Although you can use any output format with this feature,
*************** bar
*** 3241,3247 ****
list. If set to a value of <literal>ignoredups</literal>, lines
matching the previous history line are not entered. A value of
<literal>ignoreboth</literal> combines the two options. If
! unset, or if set to <literal>none</literal> (the default), all lines
read in interactive mode are saved on the history list.
</para>
<note>
--- 3274,3280 ----
list. If set to a value of <literal>ignoredups</literal>, lines
matching the previous history line are not entered. A value of
<literal>ignoreboth</literal> combines the two options. If
! set to <literal>none</literal> (the default), all lines
read in interactive mode are saved on the history list.
</para>
<note>
*************** bar
*** 3257,3264 ****
<term><varname>HISTFILE</varname></term>
<listitem>
<para>
! The file name that will be used to store the history list. The default
! value is <filename>~/.psql_history</filename>. For example, putting:
<programlisting>
\set HISTFILE ~/.psql_history- :DBNAME
</programlisting>
--- 3290,3301 ----
<term><varname>HISTFILE</varname></term>
<listitem>
<para>
! The file name that will be used to store the history list. If unset,
! the file name is taken from the <envar>PSQL_HISTORY</envar>
! environment variable. If that is not set either, the default
! is <filename>~/.psql_history</filename>,
! or <filename>%APPDATA%\postgresql\psql_history</filename> on Windows.
! For example, putting:
<programlisting>
\set HISTFILE ~/.psql_history- :DBNAME
</programlisting>
*************** bar
*** 3279,3286 ****
<term><varname>HISTSIZE</varname></term>
<listitem>
<para>
! The number of commands to store in the command history. The
! default value is 500.
</para>
<note>
<para>
--- 3316,3325 ----
<term><varname>HISTSIZE</varname></term>
<listitem>
<para>
! 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.
</para>
<note>
<para>
*************** bar
*** 3297,3303 ****
<para>
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.
</para>
</listitem>
</varlistentry>
--- 3336,3342 ----
<para>
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.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3350,3356 ****
generates an error, the error is ignored and the transaction
continues. When set to <literal>interactive</>, such errors are only
ignored in interactive sessions, and not when reading script
! files. When unset or set to <literal>off</>, a statement in a
transaction block that generates an error aborts the entire
transaction. The error rollback mode works by issuing an
implicit <command>SAVEPOINT</> for you, just before each command
--- 3389,3395 ----
generates an error, the error is ignored and the transaction
continues. When set to <literal>interactive</>, such errors are only
ignored in interactive sessions, and not when reading script
! files. When set to <literal>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 <command>SAVEPOINT</> for you, just before each command
*************** bar
*** 3385,3391 ****
<para>
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.
</para>
</listitem>
</varlistentry>
--- 3424,3430 ----
<para>
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.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3458,3464 ****
<para>
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.
</para>
</listitem>
</varlistentry>
--- 3497,3503 ----
<para>
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.
</para>
</listitem>
</varlistentry>
*************** bar
*** 3481,3487 ****
<listitem>
<para>
This variable is set at program start-up to
! reflect <application>psql</>'s version. It can be unset or changed.
</para>
</listitem>
</varlistentry>
--- 3520,3526 ----
<listitem>
<para>
This variable is set at program start-up to
! reflect <application>psql</>'s version. It can be changed or unset.
</para>
</listitem>
</varlistentry>
*************** PSQL_EDITOR_LINENUMBER_ARG='--line '
*** 4015,4020 ****
--- 4054,4060 ----
</para>
<para>
The location of the history file can be set explicitly via
+ the <varname>HISTFILE</varname> <application>psql</> variable or
the <envar>PSQL_HISTORY</envar> environment variable.
</para>
</listitem>
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 ****
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);
--- 166,173 ----
SetVariable(pset.vars, "VERSION", PG_VERSION_STR);
! /* 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 = '\0';
if (!SetVariable(pset.vars, value, equal_loc + 1))
! exit(EXIT_FAILURE);
}
free(value);
--- 576,588 ----
if (!equal_loc)
{
if (!DeleteVariable(pset.vars, value))
! exit(EXIT_FAILURE); /* error already printed */
}
else
{
*equal_loc = '\0';
if (!SetVariable(pset.vars, value, equal_loc + 1))
! exit(EXIT_FAILURE); /* error already printed */
}
free(value);
*************** showVersion(void)
*** 777,787 ****
/*
! * Assign hooks for psql variables.
*
* This isn't an amazingly good place for them, but neither is anywhere else.
*/
static bool
autocommit_hook(const char *newval)
{
--- 771,798 ----
/*
! * Substitute hooks and assign hooks for psql variables.
*
* This isn't an amazingly good place for them, but neither is anywhere else.
*/
+ static char *
+ bool_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ {
+ /* "\unset FOO" becomes "\set FOO off" */
+ newval = pg_strdup("off");
+ }
+ else if (newval[0] == '\0')
+ {
+ /* "\set FOO" becomes "\set FOO on" */
+ pg_free(newval);
+ newval = pg_strdup("on");
+ }
+ return newval;
+ }
+
static bool
autocommit_hook(const char *newval)
{
*************** fetch_count_hook(const char *newval)
*** 822,833 ****
return true;
}
static bool
echo_hook(const char *newval)
{
! if (newval == NULL)
! pset.echo = PSQL_ECHO_NONE;
! else if (pg_strcasecmp(newval, "queries") == 0)
pset.echo = PSQL_ECHO_QUERIES;
else if (pg_strcasecmp(newval, "errors") == 0)
pset.echo = PSQL_ECHO_ERRORS;
--- 833,851 ----
return true;
}
+ static char *
+ echo_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("none");
+ return newval;
+ }
+
static bool
echo_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "queries") == 0)
pset.echo = PSQL_ECHO_QUERIES;
else if (pg_strcasecmp(newval, "errors") == 0)
pset.echo = PSQL_ECHO_ERRORS;
*************** echo_hook(const char *newval)
*** 846,854 ****
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
{
--- 864,871 ----
static bool
echo_hidden_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "noexec") == 0)
pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC;
else
{
*************** echo_hidden_hook(const char *newval)
*** 868,876 ****
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
{
--- 885,892 ----
static bool
on_error_rollback_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "interactive") == 0)
pset.on_error_rollback = PSQL_ERROR_ROLLBACK_INTERACTIVE;
else
{
*************** on_error_rollback_hook(const char *newva
*** 887,898 ****
return true;
}
static bool
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;
--- 903,921 ----
return true;
}
+ static char *
+ comp_keyword_case_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("preserve-upper");
+ return newval;
+ }
+
static bool
comp_keyword_case_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! 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;
*************** comp_keyword_case_hook(const char *newva
*** 909,920 ****
return true;
}
static bool
histcontrol_hook(const char *newval)
{
! if (newval == NULL)
! pset.histcontrol = hctl_none;
! else if (pg_strcasecmp(newval, "ignorespace") == 0)
pset.histcontrol = hctl_ignorespace;
else if (pg_strcasecmp(newval, "ignoredups") == 0)
pset.histcontrol = hctl_ignoredups;
--- 932,950 ----
return true;
}
+ static char *
+ histcontrol_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("none");
+ return newval;
+ }
+
static bool
histcontrol_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "ignorespace") == 0)
pset.histcontrol = hctl_ignorespace;
else if (pg_strcasecmp(newval, "ignoredups") == 0)
pset.histcontrol = hctl_ignoredups;
*************** prompt3_hook(const char *newval)
*** 952,963 ****
return true;
}
static bool
verbosity_hook(const char *newval)
{
! if (newval == NULL)
! pset.verbosity = PQERRORS_DEFAULT;
! else if (pg_strcasecmp(newval, "default") == 0)
pset.verbosity = PQERRORS_DEFAULT;
else if (pg_strcasecmp(newval, "terse") == 0)
pset.verbosity = PQERRORS_TERSE;
--- 982,1000 ----
return true;
}
+ static char *
+ verbosity_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("default");
+ return newval;
+ }
+
static bool
verbosity_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "default") == 0)
pset.verbosity = PQERRORS_DEFAULT;
else if (pg_strcasecmp(newval, "terse") == 0)
pset.verbosity = PQERRORS_TERSE;
*************** verbosity_hook(const char *newval)
*** 974,985 ****
return true;
}
static bool
show_context_hook(const char *newval)
{
! if (newval == NULL)
! pset.show_context = PQSHOW_CONTEXT_ERRORS;
! else if (pg_strcasecmp(newval, "never") == 0)
pset.show_context = PQSHOW_CONTEXT_NEVER;
else if (pg_strcasecmp(newval, "errors") == 0)
pset.show_context = PQSHOW_CONTEXT_ERRORS;
--- 1011,1029 ----
return true;
}
+ static char *
+ show_context_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("errors");
+ return newval;
+ }
+
static bool
show_context_hook(const char *newval)
{
! Assert(newval != NULL); /* else substitute hook messed up */
! if (pg_strcasecmp(newval, "never") == 0)
pset.show_context = PQSHOW_CONTEXT_NEVER;
else if (pg_strcasecmp(newval, "errors") == 0)
pset.show_context = PQSHOW_CONTEXT_ERRORS;
*************** EstablishVariableSpace(void)
*** 1002,1021 ****
{
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);
}
--- 1046,1097 ----
{
pset.vars = CreateVariableSpace();
! 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 = pg_malloc(sizeof *ptr);
ptr->name = NULL;
ptr->value = NULL;
+ ptr->substitute_hook = NULL;
ptr->assign_hook = NULL;
ptr->next = NULL;
*************** ParseVariableBool(const char *value, con
*** 101,111 ****
size_t len;
bool valid = true;
if (value == NULL)
! {
! *result = false; /* not set -> assume "off" */
! return valid;
! }
len = strlen(value);
--- 102,110 ----
size_t len;
bool valid = true;
+ /* Treat "unset" as an empty string, which will lead to error below */
if (value == NULL)
! value = "";
len = strlen(value);
*************** ParseVariableNum(const char *value, cons
*** 152,159 ****
char *end;
long numval;
if (value == NULL)
! return false;
errno = 0;
numval = strtol(value, &end, 0);
if (errno == 0 && *end == '\0' && end != value && numval == (int) numval)
--- 151,160 ----
char *end;
long numval;
+ /* Treat "unset" as an empty string, which will lead to error below */
if (value == NULL)
! value = "";
!
errno = 0;
numval = strtol(value, &end, 0);
if (errno == 0 && *end == '\0' && end != value && numval == (int) numval)
*************** SetVariable(VariableSpace space, const c
*** 235,247 ****
if (!valid_variable_name(name))
{
psql_error("invalid variable name: \"%s\"\n", name);
return false;
}
- if (!value)
- return DeleteVariable(space, name);
-
for (previous = space, current = space->next;
current;
previous = current, current = current->next)
--- 236,248 ----
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;
}
for (previous = space, current = space->next;
current;
previous = current, current = current->next)
*************** SetVariable(VariableSpace space, const c
*** 249,262 ****
if (strcmp(current->name, name) == 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 = pg_strdup(value);
bool confirmed;
if (current->assign_hook)
confirmed = (*current->assign_hook) (new_value);
else
--- 250,269 ----
if (strcmp(current->name, name) == 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 = value ? pg_strdup(value) : NULL;
bool confirmed;
+ if (current->substitute_hook)
+ new_value = (*current->substitute_hook) (new_value);
+
if (current->assign_hook)
confirmed = (*current->assign_hook) (new_value);
else
*************** SetVariable(VariableSpace space, const c
*** 267,305 ****
if (current->value)
pg_free(current->value);
current->value = new_value;
}
! else
pg_free(new_value); /* current->value is left unchanged */
return confirmed;
}
}
! /* not present, make new entry */
! current = pg_malloc(sizeof *current);
! current->name = pg_strdup(name);
! current->value = pg_strdup(value);
! current->assign_hook = NULL;
! current->next = NULL;
! previous->next = current;
return true;
}
/*
! * 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 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 assigned.
*/
void
! SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook)
{
struct _variable *current,
*previous;
--- 274,334 ----
if (current->value)
pg_free(current->value);
current->value = new_value;
+
+ /*
+ * If we deleted the value, and there are no hooks to
+ * remember, we can discard the variable altogether.
+ */
+ if (new_value == NULL &&
+ current->substitute_hook == NULL &&
+ current->assign_hook == NULL)
+ {
+ previous->next = current->next;
+ free(current->name);
+ free(current);
+ }
}
! else if (new_value)
pg_free(new_value); /* current->value is left unchanged */
return confirmed;
}
}
! /* not present, make new entry ... unless we were asked to delete */
! if (value)
! {
! current = pg_malloc(sizeof *current);
! current->name = pg_strdup(name);
! current->value = pg_strdup(value);
! current->substitute_hook = NULL;
! current->assign_hook = NULL;
! current->next = NULL;
! previous->next = current;
! }
return true;
}
/*
! * 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 might
! * 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 value.
! * 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 assigned.
*/
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) == 0)
{
/* found entry, so update */
! current->assign_hook = hook;
! (void) (*hook) (current->value);
return;
}
}
--- 346,357 ----
if (strcmp(current->name, name) == 0)
{
/* found entry, so update */
! current->substitute_hook = shook;
! current->assign_hook = ahook;
! if (shook)
! current->value = (*shook) (current->value);
! if (ahook)
! (void) (*ahook) (current->value);
return;
}
}
*************** SetVariableAssignHook(VariableSpace spac
*** 327,336 ****
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
current->value = NULL;
! current->assign_hook = hook;
current->next = NULL;
previous->next = current;
! (void) (*hook) (NULL);
}
/*
--- 360,373 ----
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
current->value = NULL;
! current->substitute_hook = shook;
! current->assign_hook = ahook;
current->next = NULL;
previous->next = current;
! if (shook)
! current->value = (*shook) (current->value);
! if (ahook)
! (void) (*ahook) (current->value);
}
/*
*************** SetVariableBool(VariableSpace space, con
*** 351,392 ****
bool
DeleteVariable(VariableSpace space, const char *name)
{
! struct _variable *current,
! *previous;
!
! if (!space)
! return true;
!
! for (previous = space, current = space->next;
! current;
! previous = current, current = current->next)
! {
! if (strcmp(current->name, name) == 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 = 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 = current->next;
! free(current->name);
! free(current);
! }
! return true;
! }
! }
!
! return true;
}
/*
--- 388,394 ----
bool
DeleteVariable(VariableSpace space, const char *name)
{
! return SetVariable(space, name, NULL);
}
/*
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 set
* 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, 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 an assign hook function is installed with SetVariableHooks(), 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.
***************
*** 31,45 ****
typedef bool (*VariableAssignHook) (const char *newval);
/*
* Data structure representing one variable.
*
* 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.
*/
struct _variable
{
char *name;
char *value;
VariableAssignHook assign_hook;
struct _variable *next;
};
--- 31,69 ----
typedef bool (*VariableAssignHook) (const char *newval);
/*
+ * Variables can also be given "substitute hook" functions. The substitute
+ * hook can replace values (including NULL) with other values, allowing
+ * normalization of variable contents. For example, for a boolean variable,
+ * 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 before
+ * 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 return
+ * 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 values;
+ * that's a job for the assign hook.
+ *
+ * When a substitute hook is installed with SetVariableHooks(), it is applied
+ * 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);
+
+ /*
* Data structure representing one variable.
*
* 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(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);
bool SetVariable(VariableSpace space, const char *name, const char *value);
- void SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
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 */
--- 89,101 ----
void PrintVariables(VariableSpace space);
bool SetVariable(VariableSpace space, const char *name, const char *value);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
+ void SetVariableHooks(VariableSpace space, const char *name,
+ VariableSubstituteHook shook,
+ VariableAssignHook ahook);
+
void PsqlVarEnumError(const char *name, const char *value, const char *suggestions);
#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
-- \gset
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
BTW ... while I've been fooling with this issue, I've gotten a bit
annoyed at the fact that "\set" prints the variables in, essentially,
creation order. That makes the list ugly and hard to find things in,
and it exposes some psql implementation details to users. I propose
the attached simple patch to keep the list in name order instead.
(This is on top of my previous patch, but it'd be pretty trivial
to modify to apply against HEAD.)
regards, tom lane
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index b9b8fcb..9ca1000 100644
*** a/src/bin/psql/variables.c
--- b/src/bin/psql/variables.c
*************** valid_variable_name(const char *name)
*** 43,48 ****
--- 43,51 ----
/*
* A "variable space" is represented by an otherwise-unused struct _variable
* that serves as list header.
+ *
+ * The list entries are kept in name order (according to strcmp). This
+ * is mainly to make the results of PrintVariables() more pleasing.
*/
VariableSpace
CreateVariableSpace(void)
*************** GetVariable(VariableSpace space, const c
*** 74,84 ****
for (current = space->next; current; current = current->next)
{
! if (strcmp(current->name, name) == 0)
{
/* this is correct answer when value is NULL, too */
return current->value;
}
}
return NULL;
--- 77,91 ----
for (current = space->next; current; current = current->next)
{
! int cmp = strcmp(current->name, name);
!
! if (cmp == 0)
{
/* this is correct answer when value is NULL, too */
return current->value;
}
+ if (cmp > 0)
+ break; /* it's not there */
}
return NULL;
*************** SetVariable(VariableSpace space, const c
*** 247,253 ****
current;
previous = current, current = current->next)
{
! if (strcmp(current->name, name) == 0)
{
/*
* Found entry, so update, unless assign hook returns false.
--- 254,262 ----
current;
previous = current, current = current->next)
{
! int cmp = strcmp(current->name, name);
!
! if (cmp == 0)
{
/*
* Found entry, so update, unless assign hook returns false.
*************** SetVariable(VariableSpace space, const c
*** 293,298 ****
--- 302,309 ----
return confirmed;
}
+ if (cmp > 0)
+ break; /* it's not there */
}
/* not present, make new entry ... unless we were asked to delete */
*************** SetVariable(VariableSpace space, const c
*** 303,309 ****
current->value = pg_strdup(value);
current->substitute_hook = NULL;
current->assign_hook = NULL;
! current->next = NULL;
previous->next = current;
}
return true;
--- 314,320 ----
current->value = pg_strdup(value);
current->substitute_hook = NULL;
current->assign_hook = NULL;
! current->next = previous->next;
previous->next = current;
}
return true;
*************** SetVariableHooks(VariableSpace space, co
*** 343,349 ****
current;
previous = current, current = current->next)
{
! if (strcmp(current->name, name) == 0)
{
/* found entry, so update */
current->substitute_hook = shook;
--- 354,362 ----
current;
previous = current, current = current->next)
{
! int cmp = strcmp(current->name, name);
!
! if (cmp == 0)
{
/* found entry, so update */
current->substitute_hook = shook;
*************** SetVariableHooks(VariableSpace space, co
*** 354,359 ****
--- 367,374 ----
(void) (*ahook) (current->value);
return;
}
+ if (cmp > 0)
+ break; /* it's not there */
}
/* not present, make new entry */
*************** SetVariableHooks(VariableSpace space, co
*** 362,368 ****
current->value = NULL;
current->substitute_hook = shook;
current->assign_hook = ahook;
! current->next = NULL;
previous->next = current;
if (shook)
current->value = (*shook) (current->value);
--- 377,383 ----
current->value = NULL;
current->substitute_hook = shook;
current->assign_hook = ahook;
! current->next = previous->next;
previous->next = current;
if (shook)
current->value = (*shook) (current->value);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote: > I updated the documentation as well. I think this is committable if > there are not objections. That works for me. I tested and read it and did not find anything unexpected or worth objecting. "\unset var" acting as "\set var off" makes sense considering that its opposite "\set var" is now an accepted synonym of "\set var on" for the boolean built-ins. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
I wrote:
> 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 got more excited about doing that after noticing that not only would
it clean up the behavior of those particular variables, but we could get
rid of some code. First, we'd not need the rather warty GetVariableNum()
anymore, and second, we'd then be almost at the point where every control
variable has a hook, and therefore we could drop tab-complete.c's private
list of known variable names. That was only ever needed to cover the
possibility of important variables not being present in the variables
list.
So the attached proposed patch does these things:
1. Modify FETCH_COUNT to always have a defined value, like other control
variables, mainly so it will always appear in "\set" output.
2. Add hooks to force HISTSIZE to be defined and require it to have an
integer value. (I don't see any point in allowing it to be set to
non-integral values.)
3. Add hooks to force IGNOREEOF to be defined and require it to have an
integer value. Unlike the other cases, here we're trying to be
bug-compatible with a rather bogus externally-defined behavior, so I think
we need to continue to allow "\set IGNOREEOF whatever". What I propose is
that the substitution hook silently replace non-numeric values with "10",
so that the stored value always reflects what we're really doing.
4. Add a dummy assign hook for HISTFILE, just so it's always in
variables.c's list. We can't require it to be defined always, because
that would break the interaction with the PSQL_HISTORY environment
variable, so there isn't any change in visible behavior here.
5. Remove tab-complete.c's private list of known variable names. Given
the other changes, there are no control variables it won't show anyway.
This does mean that if for some reason you've unset one of the status
variables (DBNAME, HOST, etc), that variable would not appear in tab
completion for \set. But I think that's fine, for at least two reasons:
we shouldn't be encouraging people to use those variables as regular
variables, and if someone does do so anyway, why shouldn't it act just
like a regular variable?
6. Remove no-longer-used-anywhere GetVariableNum().
Any objections?
regards, tom lane
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b9c8fcc..ae58708 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** bar
*** 3247,3258 ****
fail after having already displayed some rows.
</para>
- <para>
- <varname>FETCH_COUNT</varname> 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.
- </para>
-
<tip>
<para>
Although you can use any output format with this feature,
--- 3247,3252 ----
*************** bar
*** 3316,3325 ****
<term><varname>HISTSIZE</varname></term>
<listitem>
<para>
! 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.
</para>
<note>
<para>
--- 3310,3317 ----
<term><varname>HISTSIZE</varname></term>
<listitem>
<para>
! The maximum number of commands to store in the command history
! (default 500). If set to a negative value, no limit is applied.
</para>
<note>
<para>
*************** bar
*** 3345,3357 ****
<term><varname>IGNOREEOF</varname></term>
<listitem>
<para>
! If unset, sending an <acronym>EOF</> character (usually
<keycombo action="simul"><keycap>Control</><keycap>D</></>)
to an interactive session of <application>psql</application>
! will terminate the application. If set to a numeric value,
! that many <acronym>EOF</> characters are ignored before the
! application terminates. If the variable is set but not to a
! numeric value, the default is 10.
</para>
<note>
<para>
--- 3337,3349 ----
<term><varname>IGNOREEOF</varname></term>
<listitem>
<para>
! If set to 1 or less, sending an <acronym>EOF</> character (usually
<keycombo action="simul"><keycap>Control</><keycap>D</></>)
to an interactive session of <application>psql</application>
! will terminate the application. If set to a larger numeric value,
! that many consecutive <acronym>EOF</> characters must be typed to
! make an interactive session terminate. If the variable is set to a
! non-numeric value, it is interpreted as 10.
</para>
<note>
<para>
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 5365629..3e3cab4 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** helpVariables(unsigned short int pager)
*** 348,356 ****
" (default: 0=unlimited)\n"));
fprintf(output, _(" HISTCONTROL controls command history [ignorespace, ignoredups, ignoreboth]\n"));
fprintf(output, _(" HISTFILE file name used to store the command history\n"));
! fprintf(output, _(" HISTSIZE the number of commands to store in the command history\n"));
fprintf(output, _(" HOST the currently connected database server host\n"));
! fprintf(output, _(" IGNOREEOF if unset, sending an EOF to interactive session terminates
application\n"));
fprintf(output, _(" LASTOID value of the last affected OID\n"));
fprintf(output, _(" ON_ERROR_ROLLBACK if set, an error doesn't stop a transaction (uses implicit
savepoints)\n"));
fprintf(output, _(" ON_ERROR_STOP stop batch execution after error\n"));
--- 348,356 ----
" (default: 0=unlimited)\n"));
fprintf(output, _(" HISTCONTROL controls command history [ignorespace, ignoredups, ignoreboth]\n"));
fprintf(output, _(" HISTFILE file name used to store the command history\n"));
! fprintf(output, _(" HISTSIZE max number of commands to store in the command history\n"));
fprintf(output, _(" HOST the currently connected database server host\n"));
! fprintf(output, _(" IGNOREEOF number of EOFs needed to terminate an interactive session\n"));
fprintf(output, _(" LASTOID value of the last affected OID\n"));
fprintf(output, _(" ON_ERROR_ROLLBACK if set, an error doesn't stop a transaction (uses implicit
savepoints)\n"));
fprintf(output, _(" ON_ERROR_STOP stop batch execution after error\n"));
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 3e3e97a..b8c9a00 100644
*** a/src/bin/psql/input.c
--- b/src/bin/psql/input.c
*************** finishInput(void)
*** 539,548 ****
#ifdef USE_READLINE
if (useHistory && psql_history)
{
! int hist_size;
!
! hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1);
! (void) saveHistory(psql_history, hist_size);
free(psql_history);
psql_history = NULL;
}
--- 539,545 ----
#ifdef USE_READLINE
if (useHistory && psql_history)
{
! (void) saveHistory(psql_history, pset.histsize);
free(psql_history);
psql_history = NULL;
}
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index dc25b4b..6e358e2 100644
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
*************** MainLoop(FILE *source)
*** 162,168 ****
/* This tries to mimic bash's IGNOREEOF feature. */
count_eof++;
! if (count_eof < GetVariableNum(pset.vars, "IGNOREEOF", 0, 10))
{
if (!pset.quiet)
printf(_("Use \"\\q\" to leave %s.\n"), pset.progname);
--- 162,168 ----
/* This tries to mimic bash's IGNOREEOF feature. */
count_eof++;
! if (count_eof < pset.ignoreeof)
{
if (!pset.quiet)
printf(_("Use \"\\q\" to leave %s.\n"), pset.progname);
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 4c7c3b1..195f5a1 100644
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*************** typedef struct _psqlSettings
*** 125,130 ****
--- 125,132 ----
bool singleline;
bool singlestep;
int fetch_count;
+ int histsize;
+ int ignoreeof;
PSQL_ECHO echo;
PSQL_ECHO_HIDDEN echo_hidden;
PSQL_ERROR_ROLLBACK on_error_rollback;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index a3654e6..88d686a 100644
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** showVersion(void)
*** 774,779 ****
--- 774,784 ----
* Substitute hooks and assign hooks for psql variables.
*
* This isn't an amazingly good place for them, but neither is anywhere else.
+ *
+ * By policy, every special variable that controls any psql behavior should
+ * have one or both hooks, even if they're just no-ops. This ensures that
+ * the variable will remain present in variables.c's list even when unset,
+ * which ensures that it's known to tab completion.
*/
static char *
*************** singlestep_hook(const char *newval)
*** 823,839 ****
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
static bool
fetch_count_hook(const char *newval)
{
! if (newval == NULL)
! pset.fetch_count = -1; /* default value */
! else if (!ParseVariableNum(newval, "FETCH_COUNT", &pset.fetch_count))
! return false;
return true;
}
static char *
echo_substitute_hook(char *newval)
{
if (newval == NULL)
--- 828,899 ----
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+ static char *
+ fetch_count_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("0");
+ return newval;
+ }
+
static bool
fetch_count_hook(const char *newval)
{
! return ParseVariableNum(newval, "FETCH_COUNT", &pset.fetch_count);
! }
!
! static bool
! histfile_hook(const char *newval)
! {
! /*
! * Someday we might try to validate the filename, but for now, this is
! * just a placeholder to ensure HISTFILE is known to tab completion.
! */
return true;
}
static char *
+ histsize_substitute_hook(char *newval)
+ {
+ if (newval == NULL)
+ newval = pg_strdup("500");
+ return newval;
+ }
+
+ static bool
+ histsize_hook(const char *newval)
+ {
+ return ParseVariableNum(newval, "HISTSIZE", &pset.histsize);
+ }
+
+ static char *
+ ignoreeof_substitute_hook(char *newval)
+ {
+ int dummy;
+
+ /*
+ * This tries to mimic the behavior of bash, to wit "If set, the value is
+ * the number of consecutive EOF characters which must be typed as the
+ * first characters on an input line before bash exits. If the variable
+ * exists but does not have a numeric value, or has no value, the default
+ * value is 10. If it does not exist, EOF signifies the end of input to
+ * the shell." Unlike bash, however, we insist on the stored value
+ * actually being a valid integer.
+ */
+ if (newval == NULL)
+ newval = pg_strdup("0");
+ else if (!ParseVariableNum(newval, NULL, &dummy))
+ newval = pg_strdup("10");
+ return newval;
+ }
+
+ static bool
+ ignoreeof_hook(const char *newval)
+ {
+ return ParseVariableNum(newval, "IGNOREEOF", &pset.ignoreeof);
+ }
+
+ static char *
echo_substitute_hook(char *newval)
{
if (newval == NULL)
*************** EstablishVariableSpace(void)
*** 1062,1069 ****
bool_substitute_hook,
singlestep_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
! NULL,
fetch_count_hook);
SetVariableHooks(pset.vars, "ECHO",
echo_substitute_hook,
echo_hook);
--- 1122,1138 ----
bool_substitute_hook,
singlestep_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
! fetch_count_substitute_hook,
fetch_count_hook);
+ SetVariableHooks(pset.vars, "HISTFILE",
+ NULL,
+ histfile_hook);
+ SetVariableHooks(pset.vars, "HISTSIZE",
+ histsize_substitute_hook,
+ histsize_hook);
+ SetVariableHooks(pset.vars, "IGNOREEOF",
+ ignoreeof_substitute_hook,
+ ignoreeof_hook);
SetVariableHooks(pset.vars, "ECHO",
echo_substitute_hook,
echo_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d6fffcf..6e759d0 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** append_variable_names(char ***varnames,
*** 3775,3782 ****
/*
* This function supports completion with the name of a psql variable.
* The variable names can be prefixed and suffixed with additional text
! * to support quoting usages. If need_value is true, only the variables
! * that have the set values are picked up.
*/
static char **
complete_from_variables(const char *text, const char *prefix, const char *suffix,
--- 3775,3783 ----
/*
* This function supports completion with the name of a psql variable.
* The variable names can be prefixed and suffixed with additional text
! * to support quoting usages. If need_value is true, only variables
! * that are currently set are included; otherwise, special variables
! * (those that have hooks) are included even if currently unset.
*/
static char **
complete_from_variables(const char *text, const char *prefix, const char *suffix,
*************** complete_from_variables(const char *text
*** 3789,3821 ****
int i;
struct _variable *ptr;
- static const char *const known_varnames[] = {
- "AUTOCOMMIT", "COMP_KEYWORD_CASE", "DBNAME", "ECHO", "ECHO_HIDDEN",
- "ENCODING", "FETCH_COUNT", "HISTCONTROL", "HISTFILE", "HISTSIZE",
- "HOST", "IGNOREEOF", "LASTOID", "ON_ERROR_ROLLBACK", "ON_ERROR_STOP",
- "PORT", "PROMPT1", "PROMPT2", "PROMPT3", "QUIET",
- "SHOW_CONTEXT", "SINGLELINE", "SINGLESTEP",
- "USER", "VERBOSITY", NULL
- };
-
varnames = (char **) pg_malloc((maxvars + 1) * sizeof(char *));
- if (!need_value)
- {
- for (i = 0; known_varnames[i] && nvars < maxvars; i++)
- append_variable_names(&varnames, &nvars, &maxvars,
- known_varnames[i], prefix, suffix);
- }
-
for (ptr = pset.vars->next; ptr; ptr = ptr->next)
{
if (need_value && !(ptr->value))
continue;
- for (i = 0; known_varnames[i]; i++) /* remove duplicate entry */
- {
- if (strcmp(ptr->name, known_varnames[i]) == 0)
- continue;
- }
append_variable_names(&varnames, &nvars, &maxvars, ptr->name,
prefix, suffix);
}
--- 3790,3801 ----
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 9ca1000..d9d0763 100644
*** a/src/bin/psql/variables.c
--- b/src/bin/psql/variables.c
*************** ParseVariableNum(const char *value, cons
*** 180,210 ****
}
/*
- * Read integer value of the numeric variable named "name".
- *
- * Return defaultval if it is not set, or faultval if its value is not a
- * valid integer. (No error message is issued.)
- */
- int
- GetVariableNum(VariableSpace space,
- const char *name,
- int defaultval,
- int faultval)
- {
- const char *val;
- int result;
-
- val = GetVariable(space, name);
- if (!val)
- return defaultval;
-
- if (ParseVariableNum(val, NULL, &result))
- return result;
- else
- return faultval;
- }
-
- /*
* Print values of all variables.
*/
void
--- 180,185 ----
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index 84be780..1925793 100644
*** a/src/bin/psql/variables.h
--- b/src/bin/psql/variables.h
*************** bool ParseVariableBool(const char *value
*** 81,91 ****
bool ParseVariableNum(const char *value, const char *name,
int *result);
- int GetVariableNum(VariableSpace space,
- const char *name,
- int defaultval,
- int faultval);
-
void PrintVariables(VariableSpace space);
bool SetVariable(VariableSpace space, const char *name, const char *value);
--- 81,86 ----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers