On 06/26/2014 05:07 AM, Amit Kapila wrote:
> On Wed, Jun 25, 2014 at 9:56 PM, Vik Fearing <vik.fear...@dalibo.com
> <mailto:vik.fear...@dalibo.com>> wrote:
>> On 06/25/2014 03:04 PM, Amit Kapila wrote:
>> > Currently you can achieve that by
>> > "ALTER SYSTEM RESET guc = Default;".
>> > However it will be good to have support for RESET as well. I think it
>> > should not be too complicated to implement that syntax, I personally
>> > don't have bandwidth to it immediately, but I would like to take care
>> > of it unless you or someone wants to do it by the time I get some
>> > bandwidth.
>>
>> Would something like this suffice?
>
> 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.
> About patch:
>
> + | ALTER SYSTEM_P RESET var_name
> + {
> + AlterSystemStmt *n = makeNode(AlterSystemStmt);
> + n->setstmt = makeNode(VariableSetStmt);
> + n->setstmt->kind = VAR_RESET;
> + n->setstmt->name = $4;
> + $ = (Node *)n;
> + }
>
> I think it would be better to have ALTER SYSTEM_P as generic and
> then SET | RESET as different versions, something like below:
>
> | SET reloptions
> {
> AlterTableCmd *n = makeNode(AlterTableCmd);
> n->subtype = AT_SetRelOptions;
> n->def = (Node *)$2;
> $$ = (Node *)n;
> }
> /* ALTER TABLE <name> RESET (...) */
> | RESET reloptions
> {
> AlterTableCmd *n = makeNode(AlterTableCmd);
> n->subtype = AT_ResetRelOptions;
> n->def = (Node *)$2;
> $$ = (Node *)n;
> }
>
> Another point is that if we decide to support RESET ALL syntax, then
> we might want reuse VariableResetStmt, may be by breaking into
> generic and specific like we have done for generic_set.
I didn't quite follow your ALTER TABLE example because I don't think
it's necessary, but I did split out VariableResetStmt like you suggested
because I think that is indeed cleaner.
> Thanks for working on patch.
You're welcome.
--
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,40 ----
<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
