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:

!    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.

! %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

Reply via email to