Re: [HACKERS] postgresql.auto.conf comments
On Mon, Nov 24, 2014 at 3:26 PM, Thom Brown t...@linux.com wrote: I haven't seen this mentioned anywhere (although it may have as I haven't read through the entire history of it), but would others find it useful to have ALTER SYSTEM support comments? Oh, please no. The main thing that caused us to have no way of modifying postgresql.conf via SQL for so many years is that it's not clear how you can sensibly rewrite a file with comments in it. For example, the default postgresql.conf file has stuff like this in it: #variable = someval If variable gets set to a non-default value, you might want to uncomment that line, but now you have to parse the comments, which will be tedious and error-prone and sometimes make stupid decisions: #Back in days of yore when dinosaurs ruled the earth, we had #autovacuum_naptime=1h, but that turned out to be a bad idea. # #autovacuum_naptime=1min It's hard for a non-human to know that the second one is the one that you should uncomment. There are lots of other problems that arise, too; this is just an example. It would perhaps be OK to have comments in postgresql.conf.auto if they were designated in some way that told us which specific comment was associated with which specific setting. But we need to be very careful not to design something that requires us to write a parser that can ferret out human intent from context clues. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgresql.auto.conf comments
On 24 November 2014 at 21:04, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 24, 2014 at 3:26 PM, Thom Brown t...@linux.com wrote: I haven't seen this mentioned anywhere (although it may have as I haven't read through the entire history of it), but would others find it useful to have ALTER SYSTEM support comments? Oh, please no. The main thing that caused us to have no way of modifying postgresql.conf via SQL for so many years is that it's not clear how you can sensibly rewrite a file with comments in it. For example, the default postgresql.conf file has stuff like this in it: #variable = someval If variable gets set to a non-default value, you might want to uncomment that line, but now you have to parse the comments, which will be tedious and error-prone and sometimes make stupid decisions: #Back in days of yore when dinosaurs ruled the earth, we had #autovacuum_naptime=1h, but that turned out to be a bad idea. # #autovacuum_naptime=1min I'm not sure this is an argument against supporting comments to postgresql.auto.conf since it's specifically intended not to be edited by humans, and commenting out a parameter will remove it from the file upon further ALTER SYSTEM invocations anyway. It would perhaps be OK to have comments in postgresql.conf.auto if they were designated in some way that told us which specific comment was associated with which specific setting. But we need to be very careful not to design something that requires us to write a parser that can ferret out human intent from context clues. Perhaps the parser could automatically remove any comment blocks which are followed by a blank/empty line. So if we had: - work_mem = '4MB' # Set this lower as we're using a really fast disk interace #seq_page_cost = '0.5' # Set random_page_cost lower as we're using an SSD random_page_cost = '1.0' # This line has been manually added by a human without a newline maintenance_work_mem = '1GB' # This is an orphaned comment - I would expect the next modification to the file to cause reduce it to: - work_mem = '4MB' # Set random_page_cost lower as we're using an SSD random_page_cost = 1.0 # This line has been manually added maintenance_work_mem = '1GB' - Thom
Re: [HACKERS] postgresql.auto.conf comments
On Mon, Nov 24, 2014 at 4:22 PM, Thom Brown t...@linux.com wrote: Perhaps the parser could automatically remove any comment blocks which are followed by a blank/empty line. Well, if we can agree on something, it doesn't bother me any. I'm just saying we spent years arguing about it, because we couldn't agree on anything. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgresql.auto.conf comments
* Thom Brown (t...@linux.com) wrote: On 24 November 2014 at 20:40, Stephen Frost sfr...@snowman.net wrote: I will point out that this use of COMMENT is novel though, no? Comments are normally handled as COMMENT ON blah IS 'whatever'; ALTER SYSTEM is certainly special but I'm not sure I like the idea of having some commands which support in-command COMMENT while others don't. I typed that out in my original email, thought about it, then removed it because I decided that perhaps it isn't the same class as comment as COMMENT ON uses. That affects objects, whereas this would apply to individual config parameters within a file. Also bear in mind that if someone runs: SHOW maintenance_work_mem; And sees 4GB, they may decide to add a comment based on that, even though the source of that setting isn't postgresql.auto.conf. I'd be fine with supporting two distinct object type or perhaps one object type and two sub types to allow those to be different (no clue what the actual syntax would be..). I'd actually prefer that these comments be represented in both pg_description as well as being in the actual config file- otherwise how are you going to be able to view what's there from the database before you go and change it? The fact that the information is also persisted into the actual .auto.conf file is a convenience for admins who happen to be looking there but I'd consider what's in pg_description to be the canonical source. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] postgresql.auto.conf comments
* Robert Haas (robertmh...@gmail.com) wrote: On Mon, Nov 24, 2014 at 4:22 PM, Thom Brown t...@linux.com wrote: Perhaps the parser could automatically remove any comment blocks which are followed by a blank/empty line. Well, if we can agree on something, it doesn't bother me any. I'm just saying we spent years arguing about it, because we couldn't agree on anything. I'm mystified by this whole discussion. For my 2c (and, yes, it's far too late to change most likely, but too bad) is that the entirety of postgresql.auto.conf should be generated, and generated with the catalog as the canonical source. No one should ever be modifying it directly and if they do then we act just as badly as if they go hand-modifying heap files and screw it up. We shouldn't have ever allowed postgresql.auto.conf to be the canonical source of anything. We didn't do that with pg_shadow back when we required that and I don't see any good reason to that now. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] postgresql.auto.conf comments
On Tue, Nov 25, 2014 at 1:56 AM, Thom Brown t...@linux.com wrote: Hi, I haven't seen this mentioned anywhere (although it may have as I haven't read through the entire history of it), but would others find it useful to have ALTER SYSTEM support comments? e.g. ALTER SYSTEM SET autovacuum_vacuum_scale_factor = 0.01 WITH [ INLINE | HEADLINE ] COMMENT $As most of the tables on the system are so big, we're setting this parameter lower than the default to keep bloat under more control.$; I just made up inline and headline to suggest that we could allow either: autovacuum_vacuum_scale_factor = 0.01 # As most of the tables... and # As most of the tables on the system are so big, we're setting this parameter # lower than the default to keep bloat under more control. autovacuum_vacuum_scale_factor = 0.01 I think the biggest hurdle to achieve this is to enhance/implement the parser for it, right now the function to parse config file (ParseConfigFp()) gives us the list of name-value pair item's. I think if we could improve it or write a different function which can return name-value-comment item's, then it won't be too difficult for Alter System to support it. The rationale being that it's often the case one wants to document the reason for a parameter being configured so, but there's no way of doing this for settings in postgresql.auto.conf as they'll be wiped out if added manually. I think this is completely genuine request and I have seen that other systems which support Alter System does support comments along with each entry. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
