On Sat, Aug 30, 2014 at 12:27 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Wed, Aug 27, 2014 at 7:16 PM, Fujii Masao <masao.fu...@gmail.com> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers