On Fri, Jan 03, 2020 at 01:01:18PM +0100, Fabien COELHO wrote:
> Without looking at the context I thought that argv[0] was the program name,
> which is not the case here. I put it back everywhere, including the DEBUG
> message.
The variable names in Command are confusing IMO...
> Ok. I homogeneised another similar message.
>
> Patch v3 attached hopefully fixes all of the above.
+ pg_log_error("gaussian parameter must be at least "
+ "%f (not %f)", MIN_GAUSSIAN_PARAM, param);
I would keep all the error message strings to be on the same line.
That makes grepping for them easier on the same, and that's the usual
convention even if these are larger than 72-80 characters.
#ifdef DEBUG
- printf("shell parameter name: \"%s\", value: \"%s\"\n", argv[1], res);
+ pg_log_debug("%s: shell parameter name: \"%s\", value: \"%s\"", argv[0], argv[1], res);
#endif
Worth removing this ifdef?
- fprintf(stderr, "%s", PQerrorMessage(con));
+ pg_log_fatal("unexpected copy in result");
+ pg_log_error("%s", PQerrorMessage(con));
exit(1);
[...]
- fprintf(stderr, "%s", PQerrorMessage(con));
+ pg_log_fatal("cannot count number of branches");
+ pg_log_error("%s", PQerrorMessage(con));
These are inconsistent with the rest, why not combining both?
I think that I would just remove the "debug" variable defined in
pgbench.c all together, and switch the messages for the duration and
the one in executeMetaCommand to use info-level logging..
--
Michael