On Sat, 29 Oct 2016 22:00:08 +0200 Gilles Darold <gilles.dar...@dalibo.com> wrote:
> The attached v10 of the current_logfiles patch include your last > changes on documentation but not the patch on v9 about the > user-supplied GUC value. I think the v10 path is ready for committers > and that the additional patch to add src/include/utils/guc_values.h > to define user GUC values is something that need to be taken outside > this one. Imo, thoses GUC values (stderr, csvlog) are not expected to > change so often to require a global definition, but why not, if > committers think this must be done I can add it to a v11 patch. I agree that the guc_values.h patches should be submitted separately to the committers, when your patch is submitted. Creating symbols for these values is a matter of coding style they should resolve. (IMO it's not whether the values will change, it's whether someone reading the code can read the letters "stdout" and know to what they refer and where to find related usage elsewhere in the code.) I'll keep up the guc_values.h patches and have them ready for submission along with your patch. I don't think your patch is quite ready for submission to the committers. Attached is a patch to be applied on top of your v10 patch which does basic fixup to logfile_writename(). I have some questions about logfile_writename(): Why does the logfile_open() call fail silently? Why not use ereport() here? (With a WARNING level.) Why do the ereport() invocations all have a LOG level? You're not recovering and the errors are unexpected so I'd think WARNING would be the right level. (I previously, IIRC, suggested LOG level -- only if you are retrying and recovering from an error.) Have you given any thought to my proposal to change CURRENT_LOG_FILENAME to LOG_METAINFO_FILE? I'm not sure I've looked at every bit of your patch yet. I won't have much time to do more real work until after Tuesday morning. Regards, Karl <k...@meme.com> Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
patch_pg_current_logfile-v10.diff.logfile_writename
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers