On 06/26/2014 05:07 AM, Amit Kapila wrote:
> On Wed, Jun 25, 2014 at 9:56 PM, Vik Fearing <vik.fearing@dalibo.com
> <mailto:vik.fearing@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
