Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
От | Boszormenyi Zoltan |
---|---|
Тема | Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review]) |
Дата | |
Msg-id | 51C06A01.40807@cybertec.at обсуждение исходный текст |
Ответ на | Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review]) (Amit Kapila <amit.kapila@huawei.com>) |
Ответы |
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
(Amit Kapila <amit.kapila@huawei.com>)
|
Список | pgsql-hackers |
2013-06-18 14:11 keltezéssel, Amit Kapila írta: > On Tuesday, June 18, 2013 3:26 PM Boszormenyi Zoltan wrote: >> Hi, >> review below. > Thanks for the review. > > >>>>>> There are 2 options to proceed for this patch for 9.4 >>>>>> 1. Upload the SET PERSISTENT syntax patch for coming CF by fixing >>>>>> existing >>>>>> review comments >>>>>> 2. Implement new syntax ALTER SYSTEM as proposed in below mail >>>>>> Could you suggest me what could be best way to proceed for this >>>>>> patch? >>>>> I'm still in favor of some syntax involving ALTER, because it's still >>>>> true that this behaves more like the existing GUC-setting commands >>>>> that use ALTER (which change configuration for future sessions) >>>>> rather >>>>> the ones that use SET (which change the current settings for some >>>>> period of time). > >>>> I will change the patch as per below syntax if there are no objections: >>>> ALTER SYSTEM SET configuration_parameter {TO | =} {value, | 'value'}; >>> Modified patch contains: >>> 1. Syntax implemented is >>> ALTER SYSTEM SET configuration_parameter {TO | =} {value, | 'value' | >>> DEFAULT}; >>> If user specifies DEFAULT, it will remove entry from auto conf file. >>> 2. File name to store settings set by ALTER SYSTEM command is still >>> persistent.auto.conf >>> 3. Added a new page for Alter System command in docs. In compatability >>> section, I had mentioned that >>> this statement is an PostgresQL extension. I had tried to search in >>> SQL-92 spec but couldn't find such command. >>> 4. During test of this patch, I had observed one problem for PGC_BACKEND >>> parameters like log_connections. > >> If I change these parameters directly in postgresql.conf and then do >>> pg_reload_conf() and reconnect, it will >>> still show the old value. This is observed only on WINDOWS. I will >>> investigate this problem and update you. >>> Due to this problem, I had removed few test cases. >> I can't reproduce this error under Linux (Fedora 19/x86_64). >> The bug might be somewhere else and not caused by your patch >> if manually adding/removing "log_connections = on" from postgresql.conf >> exhibits the same behaviour under Windows. (If I read it correctly, >> you tested it exactly this way.) Does the current GIT version behave >> the same way? > Yes, the current Git has problem which I had reported in below mail: > > http://www.postgresql.org/message-id/009801ce68f7$3a746340$af5d29c0$@kapila@ > huawei.com > > This problem is not related to this patch, it occurs only on WINDOWS or > under EXEC_BACKEND flag. > I think we can discuss this problem in above mail thread. > > > >> * Have all the bases been covered? >> According to the above note under Windows, not yet. >> The name "persistent.auto.conf" is not yet set in stone. >> postgresql.auto.conf might be better. > I think, the decision of name, we can leave to committer with below > possibilities, > as it is very difficult to build consensus on any particular name. > > Auto.conf > System.auto.conf > Postgresql.auto.conf > Persistent.auto.conf > >> Apply the patch, compile it and test: > >> * Does it follow the project coding guidelines? >> Mostly, yes. Some nits, though. At some places, you do: >> - ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, &tail); >> + ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, > &tail,NULL); > > I think you mean ParseConfigFp()? Sorry, yes. > > without a space between the last comma and the NULL keyword. > >> Also, in the comment above ParseConfigFile() there are unnecessary >> white space changes and spaces at the end of the line. > Do you mean to say whitespaces in below text? > > ! * and absolute-ifying the path name if necessary. > > ! * > ! * While parsing, it records if it has parsed persistent.auto.conf file. > ! * This information can be used by the callers to ensure if the parameters > ! * set by ALTER SYSTEM SET command will be effective. > */ Yes. Anyway, both would be fixed by a pgindent run. (I think.) > > With Regards, > Amit Kapila. > -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
В списке pgsql-hackers по дате отправления: