From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Specifying the log file name of pgbench -l option |
Date: | 2016-11-01 22:46:13 |
Message-ID: | alpine.DEB.2.20.1611012323250.9978@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello Masahiko,
>> So I would suggest to:
>> - fix the compilation issue
>> - leave -l/--log as it is, i.e. use "pgbench_log" as a prefix
>> - add --log-prefix=... (long option only) for changing this prefix
>
> I agree. It's better to add the separated option to specify the prefix
> of log file instead of changing the existing behaviour. Attached
> latest patch incorporated review comments.
> Please review it.
Patch applies, compiles and works for me.
This new option is for benchmarking, so "benchmarking_option_set" should
be set to true.
To simplify the code, I would suggest to initialize explicitely
"logfile_prefix" to the default value which is then overriden when the
option is set, which allows to get rid of the "prefix" variable later.
There is no reason to change the documentation by breaking a line at
another place if the text is not changed (where ... with 1).
I would suggest to simplify a little bit the documentation:
"prefix of log file ..." ->
"default log file prefix that can be changed with <option>...</>"
Otherwise the explanations seem clear enough to me. If a native English
speaker could check them though, it would be nice.
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Vik Fearing | 2016-11-02 00:01:16 | Re: Patch to implement pg_current_logfile() function |
Previous Message | Robert Haas | 2016-11-01 20:22:29 | Re: Typo in comment in contrib/postgres_fdw/deparse.c |