On 06/27/2014 06:22 AM, Amit Kapila wrote:
> On Thu, Jun 26, 2014 at 8:17 PM, Vik Fearing <vik.fear...@dalibo.com
> <mailto:vik.fear...@dalibo.com>> wrote:
>> On 06/26/2014 05:07 AM, Amit Kapila wrote:
>> > I think it will make sense if we support RESET ALL as well similar
>> > to Alter Database .. RESET ALL syntax. Do you see any reason
>> > why we shouldn't support RESET ALL syntax for Alter System?
>>
>> Yes, that makes sense. I've added that in the attached version 2 of the
>> patch.
>
> I think the idea used in patch to implement RESET ALL is sane. In passing
> by, I noticed that modified lines in .sgml have crossed 80 char
> boundary which
> is generally preferred to be maintained..
>
> Refer below modification.
>
> ! 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
I did that on purpose so that it's easy for a reviewer/committer to see
what changed.
This third patch reformats the documentation in the way I expected it to
be committed.
--
Vik
*** 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,37 **** 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.
--- 33,41 ----
<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. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 401,407 **** 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
--- 401,407 ----
%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
***************
*** 1567,1605 **** 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;
}
;
--- 1567,1613 ----
;
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;
}
;
***************
*** 8474,8480 **** DropdbStmt: DROP DATABASE database_name
/*****************************************************************************
*
! * ALTER SYSTEM SET
*
* This is used to change configuration parameters persistently.
*****************************************************************************/
--- 8482,8488 ----
/*****************************************************************************
*
! * ALTER SYSTEM
*
* This is used to change configuration parameters persistently.
*****************************************************************************/
***************
*** 8486,8491 **** AlterSystemStmt:
--- 8494,8505 ----
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
***************
*** 6693,6698 **** AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
--- 6693,6699 ----
{
char *name;
char *value;
+ bool resetall = false;
int Tmpfd = -1;
FILE *infile;
struct config_generic *record;
***************
*** 6720,6756 **** 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)));
/*
--- 6721,6768 ----
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)));
! }
/*
***************
*** 6782,6808 **** 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);
--- 6794,6828 ----
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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
