On Fri, Aug 10, 2018 at 04:46:04PM +0300, Marina Polyakova wrote: > > +1 from me to keep initial name "pgbench_error". "pgbench_log" for new > > function looks nice to me. I think it is better than just "log", > > because "log" may conflict with natural logarithmic function (see "man 3 > > log"). > > Do you think that pgbench_log (or another whose name speaks only about > logging) will look good, for example, with FATAL? Because this means that > the logging function also processes errors and calls exit(1) if necessary..
Yes, why not. "_log" just means that you want to log some message with the specified log level. Moreover those messages sometimes aren't error: pgbench_error(LOG, "starting vacuum..."); > > I agree with Fabien. Calling pgbench_error() inside pgbench_error() > > could be dangerous. I think "fmt" checking could be removed, or we may > > use Assert() > > I would like not to use Assert in this case because IIUC they are mostly > used for testing. I'd vote to remove this check at all. I don't see any place where it is possible to call pgbench_error() passing empty "fmt". -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company