Bonjour Michaël,
I don't follow what you mean by that.
The first versions of the patch did not change syntax_error(), and the
version committed has switched to use PQExpBufferData there. I think
that we should just do the same for the debug logs executing the meta
commands. This way, we get an output consistent with what's printed
out for sending or receiving stuff. Please see the attached.
Yep, I thought of it, but I was not very keen on having a malloc/free
cycle just for one debug message. However under debug this is probably not
an issue.
Your patch works for me. IT can avoid some level of format interpretation
overheads by switching to Char/Str functions, see first attachement.
The other point is the test on __pg_log_level, see second attached.
--
Fabien.
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ee1134aea2..9e1c31413a 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3297,10 +3297,20 @@ executeMetaCommand(CState *st, instr_time *now)
if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
{
- fprintf(stderr, "client %d executing \\%s", st->id, argv[0]);
+ PQExpBufferData buf;
+
+ initPQExpBuffer(&buf);
+
+ printfPQExpBuffer(&buf, "client %d executing \\%s", st->id, argv[0]);
for (int i = 1; i < argc; i++)
- fprintf(stderr, " %s", argv[i]);
- fprintf(stderr, "\n");
+ {
+ appendPQExpBufferChar(&buf, ' ');
+ appendPQExpBufferStr(&buf, argv[i]);
+ }
+
+ pg_log_debug("%s", buf.data);
+
+ termPQExpBuffer(&buf);
}
if (command->meta == META_SLEEP)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ee1134aea2..44bebed717 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3295,7 +3295,7 @@ executeMetaCommand(CState *st, instr_time *now)
argc = command->argc;
argv = command->argv;
- if (unlikely(__pg_log_level <= PG_LOG_DEBUG))
+ if (pg_log_debug_level)
{
fprintf(stderr, "client %d executing \\%s", st->id, argv[0]);
for (int i = 1; i < argc; i++)
diff --git a/src/include/common/logging.h b/src/include/common/logging.h
index 028149c7a1..c4af92a4d9 100644
--- a/src/include/common/logging.h
+++ b/src/include/common/logging.h
@@ -72,24 +72,29 @@ void pg_logging_set_locus_callback(void (*cb) (const char **filename, uint64 *l
void pg_log_generic(enum pg_log_level level, const char *pg_restrict fmt,...) pg_attribute_printf(2, 3);
void pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_list ap) pg_attribute_printf(2, 0);
+#define pg_log_fatal_level (likely(__pg_log_level <= PG_LOG_FATAL))
#define pg_log_fatal(...) do { \
- if (likely(__pg_log_level <= PG_LOG_FATAL)) pg_log_generic(PG_LOG_FATAL, __VA_ARGS__); \
+ if (pg_log_fatal_level) pg_log_generic(PG_LOG_FATAL, __VA_ARGS__); \
} while(0)
+#define pg_log_error_level (likely(__pg_log_level <= PG_LOG_ERROR))
#define pg_log_error(...) do { \
- if (likely(__pg_log_level <= PG_LOG_ERROR)) pg_log_generic(PG_LOG_ERROR, __VA_ARGS__); \
+ if (pg_log_error_level) pg_log_generic(PG_LOG_ERROR, __VA_ARGS__); \
} while(0)
+#define pg_log_warning_level (likely(__pg_log_level <= PG_LOG_WARNING))
#define pg_log_warning(...) do { \
- if (likely(__pg_log_level <= PG_LOG_WARNING)) pg_log_generic(PG_LOG_WARNING, __VA_ARGS__); \
+ if (pg_log_warning_level) pg_log_generic(PG_LOG_WARNING, __VA_ARGS__); \
} while(0)
+#define pg_log_info_level (likely(__pg_log_level <= PG_LOG_INFO))
#define pg_log_info(...) do { \
- if (likely(__pg_log_level <= PG_LOG_INFO)) pg_log_generic(PG_LOG_INFO, __VA_ARGS__); \
+ if (pg_log_info_level) pg_log_generic(PG_LOG_INFO, __VA_ARGS__); \
} while(0)
+#define pg_log_debug_level (unlikely(__pg_log_level <= PG_LOG_DEBUG))
#define pg_log_debug(...) do { \
- if (unlikely(__pg_log_level <= PG_LOG_DEBUG)) pg_log_generic(PG_LOG_DEBUG, __VA_ARGS__); \
+ if (pg_log_debug_level) pg_log_generic(PG_LOG_DEBUG, __VA_ARGS__); \
} while(0)
#endif /* COMMON_LOGGING_H */
