Skip to content

Commit fd6cd69

Browse files
tglsfdctglsfdc
committed
Clean up psql's behavior for a few more control variables.
Modify FETCH_COUNT to always have a defined value, like other control variables, mainly so it will always appear in "\set" output. 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.) 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". Fix it so that the substitution hook silently replace non-numeric values with "10", so that the stored value always reflects what we're really doing. 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. Remove tab-complete.c's private list of known variable names, since that's really a maintenance nuisance. Given the preceding 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? Remove ugly and no-longer-used-anywhere GetVariableNum(). In general, future additions of integer-valued control variables should follow the paradigm of adding an assign hook using ParseVariableNum(), so there's no reason to expect we'd need this again later. Discussion: https://postgr.es/m/17516.1485973973@sss.pgh.pa.us
1 parent 8ac0365 commit fd6cd69

File tree

9 files changed

+90
-80
lines changed

9 files changed

+90
-80
lines changed

‎doc/src/sgml/ref/psql-ref.sgml

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3247,12 +3247,6 @@ bar
32473247
fail after having already displayed some rows.
32483248
</para>
32493249

3250-
<para>
3251-
<varname>FETCH_COUNT</varname> is ignored if it is unset or does not
3252-
have a positive value. It cannot be set to a value that is not
3253-
syntactically an integer.
3254-
</para>
3255-
32563250
<tip>
32573251
<para>
32583252
Although you can use any output format with this feature,
@@ -3316,10 +3310,8 @@ bar
33163310
<term><varname>HISTSIZE</varname></term>
33173311
<listitem>
33183312
<para>
3319-
The maximum number of commands to store in the command history.
3320-
If unset, at most 500 commands are stored by default.
3321-
If set to a value that is negative or not an integer, no limit is
3322-
applied.
3313+
The maximum number of commands to store in the command history
3314+
(default 500). If set to a negative value, no limit is applied.
33233315
</para>
33243316
<note>
33253317
<para>
@@ -3345,13 +3337,13 @@ bar
33453337
<term><varname>IGNOREEOF</varname></term>
33463338
<listitem>
33473339
<para>
3348-
If unset, sending an <acronym>EOF</> character (usually
3340+
If set to 1 or less, sending an <acronym>EOF</> character (usually
33493341
<keycombo action="simul"><keycap>Control</><keycap>D</></>)
33503342
to an interactive session of <application>psql</application>
3351-
will terminate the application. If set to a numeric value,
3352-
that many <acronym>EOF</> characters are ignored before the
3353-
application terminates. If the variable is set but not to a
3354-
numeric value, the default is 10.
3343+
will terminate the application. If set to a larger numeric value,
3344+
that many consecutive <acronym>EOF</> characters must be typed to
3345+
make an interactive session terminate. If the variable is set to a
3346+
non-numeric value, it is interpreted as 10.
33553347
</para>
33563348
<note>
33573349
<para>

‎src/bin/psql/help.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,9 @@ helpVariables(unsigned short int pager)
348348
" (default: 0=unlimited)\n"));
349349
fprintf(output, _(" HISTCONTROL controls command history [ignorespace, ignoredups, ignoreboth]\n"));
350350
fprintf(output, _(" HISTFILE file name used to store the command history\n"));
351-
fprintf(output, _(" HISTSIZE the number of commands to store in the command history\n"));
351+
fprintf(output, _(" HISTSIZE max number of commands to store in the command history\n"));
352352
fprintf(output, _(" HOST the currently connected database server host\n"));
353-
fprintf(output, _(" IGNOREEOF if unset, sending an EOF to interactive session terminates application\n"));
353+
fprintf(output, _(" IGNOREEOF number of EOFs needed to terminate an interactive session\n"));
354354
fprintf(output, _(" LASTOID value of the last affected OID\n"));
355355
fprintf(output, _(" ON_ERROR_ROLLBACK if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
356356
fprintf(output, _(" ON_ERROR_STOP stop batch execution after error\n"));

‎src/bin/psql/input.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -539,10 +539,7 @@ finishInput(void)
539539
#ifdef USE_READLINE
540540
if (useHistory && psql_history)
541541
{
542-
int hist_size;
543-
544-
hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1);
545-
(void) saveHistory(psql_history, hist_size);
542+
(void) saveHistory(psql_history, pset.histsize);
546543
free(psql_history);
547544
psql_history = NULL;
548545
}

‎src/bin/psql/mainloop.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ MainLoop(FILE *source)
162162
/* This tries to mimic bash's IGNOREEOF feature. */
163163
count_eof++;
164164

165-
if (count_eof < GetVariableNum(pset.vars, "IGNOREEOF", 0, 10))
165+
if (count_eof < pset.ignoreeof)
166166
{
167167
if (!pset.quiet)
168168
printf(_("Use \"\\q\" to leave %s.\n"), pset.progname);

‎src/bin/psql/settings.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ typedef struct _psqlSettings
125125
bool singleline;
126126
bool singlestep;
127127
int fetch_count;
128+
int histsize;
129+
int ignoreeof;
128130
PSQL_ECHO echo;
129131
PSQL_ECHO_HIDDEN echo_hidden;
130132
PSQL_ERROR_ROLLBACK on_error_rollback;

‎src/bin/psql/startup.c

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,11 @@ showVersion(void)
774774
* Substitute hooks and assign hooks for psql variables.
775775
*
776776
* This isn't an amazingly good place for them, but neither is anywhere else.
777+
*
778+
* By policy, every special variable that controls any psql behavior should
779+
* have one or both hooks, even if they're just no-ops. This ensures that
780+
* the variable will remain present in variables.c's list even when unset,
781+
* which ensures that it's known to tab completion.
777782
*/
778783

779784
static char *
@@ -823,16 +828,71 @@ singlestep_hook(const char *newval)
823828
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
824829
}
825830

831+
static char *
832+
fetch_count_substitute_hook(char *newval)
833+
{
834+
if (newval == NULL)
835+
newval = pg_strdup("0");
836+
return newval;
837+
}
838+
826839
static bool
827840
fetch_count_hook(const char *newval)
828841
{
829-
if (newval == NULL)
830-
pset.fetch_count = -1; /* default value */
831-
else if (!ParseVariableNum(newval, "FETCH_COUNT", &pset.fetch_count))
832-
return false;
842+
return ParseVariableNum(newval, "FETCH_COUNT", &pset.fetch_count);
843+
}
844+
845+
static bool
846+
histfile_hook(const char *newval)
847+
{
848+
/*
849+
* Someday we might try to validate the filename, but for now, this is
850+
* just a placeholder to ensure HISTFILE is known to tab completion.
851+
*/
833852
return true;
834853
}
835854

855+
static char *
856+
histsize_substitute_hook(char *newval)
857+
{
858+
if (newval == NULL)
859+
newval = pg_strdup("500");
860+
return newval;
861+
}
862+
863+
static bool
864+
histsize_hook(const char *newval)
865+
{
866+
return ParseVariableNum(newval, "HISTSIZE", &pset.histsize);
867+
}
868+
869+
static char *
870+
ignoreeof_substitute_hook(char *newval)
871+
{
872+
int dummy;
873+
874+
/*
875+
* This tries to mimic the behavior of bash, to wit "If set, the value is
876+
* the number of consecutive EOF characters which must be typed as the
877+
* first characters on an input line before bash exits. If the variable
878+
* exists but does not have a numeric value, or has no value, the default
879+
* value is 10. If it does not exist, EOF signifies the end of input to
880+
* the shell." Unlike bash, however, we insist on the stored value
881+
* actually being a valid integer.
882+
*/
883+
if (newval == NULL)
884+
newval = pg_strdup("0");
885+
else if (!ParseVariableNum(newval, NULL, &dummy))
886+
newval = pg_strdup("10");
887+
return newval;
888+
}
889+
890+
static bool
891+
ignoreeof_hook(const char *newval)
892+
{
893+
return ParseVariableNum(newval, "IGNOREEOF", &pset.ignoreeof);
894+
}
895+
836896
static char *
837897
echo_substitute_hook(char *newval)
838898
{
@@ -1062,8 +1122,17 @@ EstablishVariableSpace(void)
10621122
bool_substitute_hook,
10631123
singlestep_hook);
10641124
SetVariableHooks(pset.vars, "FETCH_COUNT",
1065-
NULL,
1125+
fetch_count_substitute_hook,
10661126
fetch_count_hook);
1127+
SetVariableHooks(pset.vars, "HISTFILE",
1128+
NULL,
1129+
histfile_hook);
1130+
SetVariableHooks(pset.vars, "HISTSIZE",
1131+
histsize_substitute_hook,
1132+
histsize_hook);
1133+
SetVariableHooks(pset.vars, "IGNOREEOF",
1134+
ignoreeof_substitute_hook,
1135+
ignoreeof_hook);
10671136
SetVariableHooks(pset.vars, "ECHO",
10681137
echo_substitute_hook,
10691138
echo_hook);

‎src/bin/psql/tab-complete.c

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3775,8 +3775,9 @@ append_variable_names(char ***varnames, int *nvars,
37753775
/*
37763776
* This function supports completion with the name of a psql variable.
37773777
* The variable names can be prefixed and suffixed with additional text
3778-
* to support quoting usages. If need_value is true, only the variables
3779-
* that have the set values are picked up.
3778+
* to support quoting usages. If need_value is true, only variables
3779+
* that are currently set are included; otherwise, special variables
3780+
* (those that have hooks) are included even if currently unset.
37803781
*/
37813782
static char **
37823783
complete_from_variables(const char *text, const char *prefix, const char *suffix,
@@ -3789,33 +3790,12 @@ complete_from_variables(const char *text, const char *prefix, const char *suffix
37893790
int i;
37903791
struct _variable *ptr;
37913792

3792-
static const char *const known_varnames[] = {
3793-
"AUTOCOMMIT", "COMP_KEYWORD_CASE", "DBNAME", "ECHO", "ECHO_HIDDEN",
3794-
"ENCODING", "FETCH_COUNT", "HISTCONTROL", "HISTFILE", "HISTSIZE",
3795-
"HOST", "IGNOREEOF", "LASTOID", "ON_ERROR_ROLLBACK", "ON_ERROR_STOP",
3796-
"PORT", "PROMPT1", "PROMPT2", "PROMPT3", "QUIET",
3797-
"SHOW_CONTEXT", "SINGLELINE", "SINGLESTEP",
3798-
"USER", "VERBOSITY", NULL
3799-
};
3800-
38013793
varnames = (char **) pg_malloc((maxvars + 1) * sizeof(char *));
38023794

3803-
if (!need_value)
3804-
{
3805-
for (i = 0; known_varnames[i] && nvars < maxvars; i++)
3806-
append_variable_names(&varnames, &nvars, &maxvars,
3807-
known_varnames[i], prefix, suffix);
3808-
}
3809-
38103795
for (ptr = pset.vars->next; ptr; ptr = ptr->next)
38113796
{
38123797
if (need_value && !(ptr->value))
38133798
continue;
3814-
for (i = 0; known_varnames[i]; i++) /* remove duplicate entry */
3815-
{
3816-
if (strcmp(ptr->name, known_varnames[i]) == 0)
3817-
continue;
3818-
}
38193799
append_variable_names(&varnames, &nvars, &maxvars, ptr->name,
38203800
prefix, suffix);
38213801
}

‎src/bin/psql/variables.c

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -179,31 +179,6 @@ ParseVariableNum(const char *value, const char *name, int *result)
179179
}
180180
}
181181

182-
/*
183-
* Read integer value of the numeric variable named "name".
184-
*
185-
* Return defaultval if it is not set, or faultval if its value is not a
186-
* valid integer. (No error message is issued.)
187-
*/
188-
int
189-
GetVariableNum(VariableSpace space,
190-
const char *name,
191-
int defaultval,
192-
int faultval)
193-
{
194-
const char *val;
195-
int result;
196-
197-
val = GetVariable(space, name);
198-
if (!val)
199-
return defaultval;
200-
201-
if (ParseVariableNum(val, NULL, &result))
202-
return result;
203-
else
204-
return faultval;
205-
}
206-
207182
/*
208183
* Print values of all variables.
209184
*/

‎src/bin/psql/variables.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,6 @@ bool ParseVariableBool(const char *value, const char *name,
8181
bool ParseVariableNum(const char *value, const char *name,
8282
int *result);
8383

84-
int GetVariableNum(VariableSpace space,
85-
const char *name,
86-
int defaultval,
87-
int faultval);
88-
8984
void PrintVariables(VariableSpace space);
9085

9186
bool SetVariable(VariableSpace space, const char *name, const char *value);

0 commit comments

Comments
 (0)