On Thu, Jun 26, 2014 at 8:17 PM, Vik Fearing <vik.fear...@dalibo.com> wrote: > I didn't quite follow your ALTER TABLE example because I don't think > it's necessary, I was asking to split the ALTER SYSTEM command like it's there for ALTER TABLE (AlterTableStmt: ALTER TABLE relation_expr alter_table_cmds). It would have make adding further commands to ALTER SYSTEM bit simpler and systemetic. However as there is no correctness issue here, so lets leave it like you have currently done in patch.
I have verified the patch and found that it works well for all scenario's. Few minor suggestions: 1. ! 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 It would be better if we can write a separate line for RESET ALL as is written in case of both Alter Database and Alter Role in their respective documentation. 2. ! %type <vsetstmt> generic_set set_rest set_rest_more generic_reset reset_rest SetResetClause FunctionSetResetClause Good to break it into 2 lines. 3. I think we can add some text on top of function AlterSystemSetConfigFile() to explain functionality w.r.t reset all. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
