| From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
|---|---|
| To: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Enhancing pgbench parameter checking |
| Date: | 2014-08-07 15:25:28 |
| Message-ID: | alpine.DEB.2.10.1408071659140.7564@sto |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello Tatsuo-san,
> Thanks for the review. I have registered it to Aug Commit fest.
> https://commitfest.postgresql.org/action/patch_view?id=1532
>
>> I'm not sure of the variable name "is_non_init_parameter_set". I would
>> suggest "benchmarking_option_set"?
>
> Ok, I will replace the variable name as you suggested.
>
>> Also, to be consistent, maybe it should check that no initialization-specific option are set when benchmarking.
>
> Good suggesition. Here is the v2 patch.
I applied it without problem and tested it.
* It seems that -c is ignored, the atoi() line has been removed.
* Option -q is initialization specific, but not detected as such like the
other, although there is a specific detection later. I think that it would
be better to use the systematic approach, and to remove the specific
check.
* I would name the second boolean "initialization_option_set", as it is
describe like that in the documentation.
* I would suggest the following error messages:
"some options cannot be used in initialization (-i) mode\n" and
"some options cannot be used in benchmarking mode\n".
Although these messages are rough, I think that they are enough and avoid
running something unexpected, which is your purpose.
Find attached a patch which adds these changes to your current version.
--
Fabien.
| Attachment | Content-Type | Size |
|---|---|---|
| pgbench-v2+.patch | text/x-diff | 2.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bruce Momjian | 2014-08-07 15:29:13 | Re: Proposal: Incremental Backup |
| Previous Message | Robert Haas | 2014-08-07 15:07:23 | Re: A worst case for qsort |