On Fri, 28 Oct 2016 10:03:37 +0200
Gilles Darold <gilles.dar...@dalibo.com> wrote:

> ...
> the v9 of the patch, attached here.

I notice that there are a number of user-supplied GUC
values for log_destination that are repeatedly used,
both in the GUC code and in your patch.  These are
presently written as hardcoded strings.

Attached are 2 patches which abstract the values a
user is supposed to supply for log_destination.


patch_pg_current_logfile-v9.diff.guc_values-part1
This applies to both master HEAD and on top of your v9
patch.  It abstracts the user-supplied values within
the GUC code.

patch_pg_current_logfile-v9.diff.guc_values-part2
This applies on top of your v9 patch.

I couldn't find a good place to put the newly defined symbols
in the existing code so the part1 patch creates
src/include/utils/guc_values.h.  Someone who knows
the code better than me would be better able to judge
if making a new .h file is a good idea.  Likewise, I presume
that a "GUCV_" prefix for the new symbols is good, but this
too could use review.

The odd part about the part1 patch is that GUCV_EVENTLOG
is never used anywhere but in src/backend/utils/misc/guc.c.
But it is used twice there and it seemed like as long as
I was doing the rest of the log_destination values I should
abstract eventlog too.

If we use these patches I propose that we keep the
part1 patch and submit it separately to the committers.
Seems like it'd be easier to review/commit when the changes to
existing code are kept separate from new code.

> Thanks a lot.

Thank you also for considering my ideas.  :)

Regards,

Karl <k...@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

Attachment: patch_pg_current_logfile-v9.diff.guc_values-part2
Description: Binary data

Attachment: patch_pg_current_logfile-v9.diff.guc_values-part1
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

Reply via email to