On Thursday, November 22, 2012 10:09 PM Fujii Masao wrote:
> On Thu, Nov 22, 2012 at 9:38 PM, Amit kapila <amit.kapila@huawei.com>
> wrote:
> > Patch to implement SET PERSISTENT command is attached with this mail.
> > Now it can be reviewed.
>
> I got the following compile warnings:
> xact.c:1847: warning: implicit declaration of function
> 'AtEOXact_UpdateAutoConfFiles'
> guc.c:5134: warning: enumeration value 'PGC_ENUM' not handled in switch
>
> "SET PERSISTENT name TO value" failed, though "SET PERSISTENT name =
> value"
> succeeded.
>
> =# SET PERSISTENT synchronous_commit TO 'local';
> ERROR: syntax error at or near "TO"
> LINE 1: SET PERSISTENT synchronous_commit TO 'local';
> =# SET PERSISTENT synchronous_commit = 'local';
> SET
>
> SET PERSISTENT succeeded even if invalid setting value was set.
>
> =# SET PERSISTENT synchronous_commit = 'hoge';
> SET
>
> SET PERSISTENT syntax should be explained by "\help SET" psql command.
Thank you. I shall take care of above in next patch and do more test.
>
> When I remove postgresql.auto.conf, SET PERSISTENT failed.
>
> =# SET PERSISTENT synchronous_commit = 'local';
> ERROR: failed to open "postgresql.auto.conf" file
There can be 2 ways to handle this, either we recreate the
"postgresql.auto.conf" file or give error.
I am not sure if user tries to delete internal files, what should be exact
behavior?
Any suggestion?
> We should implement "RESET PERSISTENT"? Otherwise, there is no way
> to get rid of the parameter setting from postgresql.auto.conf, via SQL.
> Also
> We should implement "SET PERSISTENT name TO DEFAULT"?
Till now, I have not implemented this in patch, thinking that it can be done
as a 2nd part if basic stuff is ready.
However I think you are right without one of "RESET PERSISTENT" or "SET
PERSISTENT name TO DEFAULT", it is difficult for user
to get rid of parameter.
Will "SET PERSISTENT name TO DEFAULT" be sufficient or do you think both are
necessary, because RESET PERSISTENT also internally might need
to behave similarly.
For implementation of "SET PERSISTENT name TO DEFAULT", there can be 2 ways
1) Delete the entry from postgresql.auto.conf
2) Update the entry value in postgresql.auto.conf to default value
Incase we just do update, then user might not be able to modify
postgresql.conf manually once the command is executed.
So I think Delete is better option. Let me know if you think otherwise or
you have some other idea to achieve it.
> Is it helpful to output the notice message like 'Run pg_reload_conf() or
> restart the server if you want new settings to take effect' always after
> SET PERSISTENT command?
Not sure because if someone uses SET PERSISTENT command, he should know the
effect or behavior of same.
I think it's good to put this in UM along with "PERSISTENT" option
explanation.
With Regards,
Amit Kapila.
