On Sat, Aug 30, 2014 at 12:27 PM, Amit Kapila <[email protected]> wrote:
> On Wed, Aug 27, 2014 at 7:16 PM, Fujii Masao <[email protected]> wrote:
>> The patch looks good to me. One minor comment is; probably you need to
>> update the tab-completion code.
>
> Thanks for the review. I have updated the patch to support
> tab-completion.
> As this is a relatively minor change, I will mark it as
> "Ready For Committer" rather than "Needs Review".
Thanks for updating the patch!
One more minor comment is; what about applying the following change
for the tab-completion for RESET ALL? This causes the tab-completion of
even ALTER SYSTEM SET to display "all" and that's strange. But
the tab-completion of "SET" has already had the same problem. So
I think that we can live with that. Attached is the patch that I added
the following change onto your patch. Barring any objection, I will commit
the patch.
@@ -545,7 +545,8 @@ static const SchemaQuery Query_for_list_of_matviews = {
"SELECT name FROM "\
" (SELECT pg_catalog.lower(name) AS name FROM pg_catalog.pg_settings "\
" WHERE context != 'internal') ss "\
-" WHERE substring(name,1,%d)='%s'"
+" WHERE substring(name,1,%d)='%s'"\
+" UNION ALL SELECT 'all' ss"
Regards,
--
Fujii Masao
*** a/doc/src/sgml/ref/alter_system.sgml
--- b/doc/src/sgml/ref/alter_system.sgml
***************
*** 22,27 **** PostgreSQL documentation
--- 22,30 ----
<refsynopsisdiv>
<synopsis>
ALTER SYSTEM SET <replaceable class="PARAMETER">configuration_parameter</replaceable> { TO | = } { <replaceable class="PARAMETER">value</replaceable> | '<replaceable class="PARAMETER">value</replaceable>' | DEFAULT }
+
+ ALTER SYSTEM RESET <replaceable class="PARAMETER">configuration_parameter</replaceable>
+ ALTER SYSTEM RESET ALL
</synopsis>
</refsynopsisdiv>
***************
*** 30,39 **** ALTER SYSTEM SET <replaceable class="PARAMETER">configuration_parameter</replace
<para>
<command>ALTER SYSTEM</command> writes the configuration parameter
! values to the <filename>postgresql.auto.conf</filename> file. With
! <literal>DEFAULT</literal>, it removes a configuration entry from
! <filename>postgresql.auto.conf</filename> file. The values will be
! effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
</para>
--- 33,44 ----
<para>
<command>ALTER SYSTEM</command> writes the configuration parameter
! values to the <filename>postgresql.auto.conf</filename> file.
! Setting the parameter to <literal>DEFAULT</literal>, or using the
! <command>RESET</command> variant, removes the configuration entry from
! <filename>postgresql.auto.conf</filename> file. Use <literal>RESET
! ALL</literal> to clear all configuration entries. The values will
! be effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
</para>
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 411,417 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <istmt> insert_rest
! %type <vsetstmt> generic_set set_rest set_rest_more SetResetClause FunctionSetResetClause
%type <node> TableElement TypedTableElement ConstraintElem TableFuncElement
%type <node> columnDef columnOptions
--- 411,418 ----
%type <istmt> insert_rest
! %type <vsetstmt> generic_set set_rest set_rest_more generic_reset reset_rest
! SetResetClause FunctionSetResetClause
%type <node> TableElement TypedTableElement ConstraintElem TableFuncElement
%type <node> columnDef columnOptions
***************
*** 1579,1617 **** NonReservedWord_or_Sconst:
;
VariableResetStmt:
! RESET var_name
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = $2;
! $$ = (Node *) n;
}
! | RESET TIME ZONE
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "timezone";
! $$ = (Node *) n;
}
! | RESET TRANSACTION ISOLATION LEVEL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "transaction_isolation";
! $$ = (Node *) n;
}
! | RESET SESSION AUTHORIZATION
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "session_authorization";
! $$ = (Node *) n;
}
! | RESET ALL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET_ALL;
! $$ = (Node *) n;
}
;
--- 1580,1626 ----
;
VariableResetStmt:
! RESET reset_rest { $$ = (Node *) $2; }
! ;
!
! reset_rest:
! generic_reset { $$ = $1; }
! | TIME ZONE
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "timezone";
! $$ = n;
}
! | TRANSACTION ISOLATION LEVEL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "transaction_isolation";
! $$ = n;
}
! | SESSION AUTHORIZATION
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = "session_authorization";
! $$ = n;
}
! ;
!
! generic_reset:
! var_name
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET;
! n->name = $1;
! $$ = n;
}
! | ALL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->kind = VAR_RESET_ALL;
! $$ = n;
}
;
***************
*** 8494,8500 **** DropdbStmt: DROP DATABASE database_name
/*****************************************************************************
*
! * ALTER SYSTEM SET
*
* This is used to change configuration parameters persistently.
*****************************************************************************/
--- 8503,8509 ----
/*****************************************************************************
*
! * ALTER SYSTEM
*
* This is used to change configuration parameters persistently.
*****************************************************************************/
***************
*** 8506,8511 **** AlterSystemStmt:
--- 8515,8526 ----
n->setstmt = $4;
$$ = (Node *)n;
}
+ | ALTER SYSTEM_P RESET generic_reset
+ {
+ AlterSystemStmt *n = makeNode(AlterSystemStmt);
+ n->setstmt = $4;
+ $$ = (Node *)n;
+ }
;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 6696,6701 **** replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p,
--- 6696,6703 ----
* This function takes all previous configuration parameters
* set by ALTER SYSTEM command and the currently set ones
* and write them all to the automatic configuration file.
+ * It just writes an empty file incase user wants to reset
+ * all the parameters.
*
* The configuration parameters are written to a temporary
* file then renamed to the final name.
***************
*** 6710,6715 **** AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
--- 6712,6718 ----
{
char *name;
char *value;
+ bool resetall = false;
int Tmpfd = -1;
FILE *infile;
struct config_generic *record;
***************
*** 6737,6773 **** AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
break;
case VAR_SET_DEFAULT:
value = NULL;
break;
default:
elog(ERROR, "unrecognized alter system stmt type: %d",
altersysstmt->setstmt->kind);
break;
}
! record = find_option(name, false, LOG);
! if (record == NULL)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("unrecognized configuration parameter \"%s\"", name)));
! /*
! * Don't allow the parameters which can't be set in configuration
! * files to be set in PG_AUTOCONF_FILENAME file.
! */
! if ((record->context == PGC_INTERNAL) ||
! (record->flags & GUC_DISALLOW_IN_FILE) ||
! (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
! ereport(ERROR,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed",
! name)));
!
! if (!validate_conf_option(record, name, value, PGC_S_FILE,
! ERROR, true, NULL,
! &newextra))
! ereport(ERROR,
! (errmsg("invalid value for parameter \"%s\": \"%s\"", name, value)));
/*
--- 6740,6787 ----
break;
case VAR_SET_DEFAULT:
+ case VAR_RESET:
+ value = NULL;
+ break;
+
+ case VAR_RESET_ALL:
value = NULL;
+ resetall = true;
break;
+
default:
elog(ERROR, "unrecognized alter system stmt type: %d",
altersysstmt->setstmt->kind);
break;
}
! /* If we're resetting everything, there's no need to validate anything */
! if (!resetall)
! {
! record = find_option(name, false, LOG);
! if (record == NULL)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("unrecognized configuration parameter \"%s\"", name)));
! /*
! * Don't allow the parameters which can't be set in configuration
! * files to be set in PG_AUTOCONF_FILENAME file.
! */
! if ((record->context == PGC_INTERNAL) ||
! (record->flags & GUC_DISALLOW_IN_FILE) ||
! (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
! ereport(ERROR,
! (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
! errmsg("parameter \"%s\" cannot be changed",
! name)));
!
! if (!validate_conf_option(record, name, value, PGC_S_FILE,
! ERROR, true, NULL,
! &newextra))
! ereport(ERROR,
! (errmsg("invalid value for parameter \"%s\": \"%s\"", name, value)));
! }
/*
***************
*** 6799,6824 **** AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
PG_TRY();
{
! if (stat(AutoConfFileName, &st) == 0)
{
! /* open file PG_AUTOCONF_FILENAME */
! infile = AllocateFile(AutoConfFileName, "r");
! if (infile == NULL)
! ereport(ERROR,
! (errmsg("failed to open auto conf file \"%s\": %m ",
! AutoConfFileName)));
! /* parse it */
! ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail);
! FreeFile(infile);
! }
! /*
! * replace with new value if the configuration parameter already
! * exists OR add it as a new cofiguration parameter in the file.
! */
! replace_auto_config_value(&head, &tail, AutoConfFileName, name, value);
/* Write and sync the new contents to the temporary file */
write_auto_conf_file(Tmpfd, AutoConfTmpFileName, &head);
--- 6813,6846 ----
PG_TRY();
{
! /*
! * If we're going to reset everything, then don't open the file, don't
! * parse it, and don't do anything with the configuration list. Just
! * write out an empty file.
! */
! if (!resetall)
{
! if (stat(AutoConfFileName, &st) == 0)
! {
! /* open file PG_AUTOCONF_FILENAME */
! infile = AllocateFile(AutoConfFileName, "r");
! if (infile == NULL)
! ereport(ERROR,
! (errmsg("failed to open auto conf file \"%s\": %m ",
! AutoConfFileName)));
! /* parse it */
! ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail);
! FreeFile(infile);
! }
! /*
! * replace with new value if the configuration parameter already
! * exists OR add it as a new cofiguration parameter in the file.
! */
! replace_auto_config_value(&head, &tail, AutoConfFileName, name, value);
! }
/* Write and sync the new contents to the temporary file */
write_auto_conf_file(Tmpfd, AutoConfTmpFileName, &head);
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 545,551 **** static const SchemaQuery Query_for_list_of_matviews = {
"SELECT name FROM "\
" (SELECT pg_catalog.lower(name) AS name FROM pg_catalog.pg_settings "\
" WHERE context != 'internal') ss "\
! " WHERE substring(name,1,%d)='%s'"
#define Query_for_list_of_set_vars \
"SELECT name FROM "\
--- 545,552 ----
"SELECT name FROM "\
" (SELECT pg_catalog.lower(name) AS name FROM pg_catalog.pg_settings "\
" WHERE context != 'internal') ss "\
! " WHERE substring(name,1,%d)='%s'"\
! " UNION ALL SELECT 'all' ss"
#define Query_for_list_of_set_vars \
"SELECT name FROM "\
***************
*** 963,969 **** psql_completion(const char *text, int start, int end)
{"AGGREGATE", "COLLATION", "CONVERSION", "DATABASE", "DEFAULT PRIVILEGES", "DOMAIN",
"EVENT TRIGGER", "EXTENSION", "FOREIGN DATA WRAPPER", "FOREIGN TABLE", "FUNCTION",
"GROUP", "INDEX", "LANGUAGE", "LARGE OBJECT", "MATERIALIZED VIEW", "OPERATOR",
! "ROLE", "RULE", "SCHEMA", "SERVER", "SEQUENCE", "SYSTEM SET", "TABLE",
"TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE",
"USER", "USER MAPPING FOR", "VIEW", NULL};
--- 964,970 ----
{"AGGREGATE", "COLLATION", "CONVERSION", "DATABASE", "DEFAULT PRIVILEGES", "DOMAIN",
"EVENT TRIGGER", "EXTENSION", "FOREIGN DATA WRAPPER", "FOREIGN TABLE", "FUNCTION",
"GROUP", "INDEX", "LANGUAGE", "LARGE OBJECT", "MATERIALIZED VIEW", "OPERATOR",
! "ROLE", "RULE", "SCHEMA", "SERVER", "SEQUENCE", "SYSTEM", "TABLE",
"TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE",
"USER", "USER MAPPING FOR", "VIEW", NULL};
***************
*** 1328,1337 **** psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST(list_ALTER_SERVER);
}
! /* ALTER SYSTEM SET <name> */
else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
pg_strcasecmp(prev2_wd, "SYSTEM") == 0 &&
! pg_strcasecmp(prev_wd, "SET") == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
/* ALTER VIEW <name> */
else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
--- 1329,1348 ----
COMPLETE_WITH_LIST(list_ALTER_SERVER);
}
! /* ALTER SYSTEM SET, RESET, RESET ALL */
! else if (pg_strcasecmp(prev2_wd, "ALTER") == 0 &&
! pg_strcasecmp(prev_wd, "SYSTEM") == 0)
! {
! static const char *const list_ALTERSYSTEM[] =
! {"SET", "RESET", NULL};
!
! COMPLETE_WITH_LIST(list_ALTERSYSTEM);
! }
! /* ALTER SYSTEM SET|RESET <name> */
else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
pg_strcasecmp(prev2_wd, "SYSTEM") == 0 &&
! (pg_strcasecmp(prev_wd, "SET") == 0 ||
! pg_strcasecmp(prev_wd, "RESET") == 0))
COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
/* ALTER VIEW <name> */
else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
