Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

From: Amit kapila <amit(dot)kapila(at)huawei(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Date: 2013-06-22 04:39:22
Message-ID: 6C0B27F7206C9E4CA54AE035729E9C38421B7C34@szxeml558-mbs.china.huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Friday, June 21, 2013 11:43 PM Robert Haas wrote:
On Fri, Jun 21, 2013 at 12:56 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Sat, Jun 22, 2013 at 12:11 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Wed, Jun 19, 2013 at 1:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>>> On 6/7/13 12:14 AM, Amit Kapila wrote:
>>>>> I will change the patch as per below syntax if there are no objections:
>>>>>
>>>>> ALTER SYSTEM SET configuration_parameter {TO | =} {value, | 'value'};
>>>>
>>>> I do like using ALTER SYSTEM in general, but now that I think about it,
>>>> the 9.3 feature to create global settings in pg_db_role_setting would
>>>> also have been a candidate for exactly that same syntax if it had been
>>>> available. In fact, if we do add ALTER SYSTEM, it might make sense to
>>>> recast that feature into that syntax.
>>>>
>>>> It might be clearer to do something like ALTER SYSTEM SET EXTERNAL FILE
>>>> or something like that. It's only a small syntax change, so don't worry
>>>> about it too much, but let's keep thinking about it.
>>>
>>> I think that anything we want to add should either go before SET or
>>> after the value, such as:
>>>
>>> ALTER SYSTEM SET configuration_parameter = 'value' IN FILE 'myconf.conf';
>>> ALTER SYSTEM CONFIGURATION FILE SET configuration_parameter = 'file';
>>> ALTER SYSTEM CONFIGURATION FILE 'myconf.conf' SET
>>> configuration_parameter = 'file';
>>>
>>> I agree we shouldn't back ourselves into a syntactic corner.
>>
>> Maybe this idea has been already discussed, but why don't we just
>> add the function like update_config_file(parameter, value)? We can
>> commit the core of the patch with that function API first, and then
>> we can discuss the syntax and add another API like ALTER SYSTEM
>> later.
>>
>> We now have set_config() function to set the parameter,
>> so adding the function for this feature should not be a surprise.
>> If the name of the function is, for example, update_conf_file,
>> most users would think that it's intuitive even if SIGHUP is not
>> automatically sent immediately. We don't need to emit
>> the message like 'This setting change will not be loaded until SIGHUP'.

> I could certainly support that plan. I'm satisfied with the ALTER
> SYSTEM syntax and feel that might find other plausible uses, but
> functional notation would be OK with me, too.

I can update the patch to have a function as suggested by Fujii-san if it can be decided
that for the first committed version it will be sufficient.
OTOH already we already have consensus on ALTER SYSTEM syntax apart from few extra keywords to make it more meaningful/extendible.
I think we can consider the current syntax (ALTER SYSTEM SET ..) and make that behavior as default for writing to auto file.
In future we can extend it with other keywords depending on usage.

With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2013-06-22 06:24:01 Re: WITH CHECK OPTION for auto-updatable views
Previous Message Amit kapila 2013-06-22 04:17:17 Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])