On Wed, Mar 10, 2021 at 1:31 AM Michael Paquier <mich...@paquier.xyz> wrote: > On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote: > > Let's move this patch forward. Based on the responses, I agree the > > default behavior should be to remove the temp files, and I think we > > should have the GUC (on the off chance that someone wants to preserve > > the temporary files for debugging or whatever other reason). > > Thanks for taking care of this. I am having some second-thoughts > about changing this behavior by default, still that's much more useful > this way.
+1 for having it on by default. I was also just looking at this patch and came here to say LGTM except for two cosmetic things, below. > > I propose to rename the GUC to remove_temp_files_after_crash, I think > > "remove" is a bit clearer than "cleanup". I've also reworded the sgml > > docs a little bit. > > "remove" sounds fine to me. +1 > > Attached is a patch with those changes. Barring objections, I'll get > > this committed in the next couple days. > > + When set to on, <productname>PostgreSQL</productname> will > automatically > Nit: using a <literal> markup for the "on" value. Maybe should say "which is the default", like other similar things? > +#remove_temp_files_after_crash = on # remove temporary files after > +# # backend crash? > The indentation of the second line is incorrect here (Incorrect number > of spaces in tabs perhaps?), and there is no need for the '#' at the > beginning of the line. Yeah, that's wrong. For some reason that one file uses a tab size of 8, unlike the rest of the tree (I guess because people will read that file in software with the more common setting of 8). If you do :set tabstop=8 in vim, suddenly it all makes sense, but it is revealed that this patch has it wrong, as you said. (Perhaps this file should have some of those special Vim/Emacs control messages so we don't keep getting this wrong?)
