Re: Truncate logs by max_log_size

On Sat, Apr 4, 2026 at 6:59 AM Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> wrote:
> v9 attached.

Thanks for updating the patch!

I tried to review this in time for the feature freeze, but unfortunately
didn't make it. Still, I'd like to share some comments for future work.

+ If greater than zero, each statement written to the server log
+ is truncated to at most this many bytes.

It would be good to clarify which query logging this parameter affects.
As I understand it, it applies only to statements logged by log_statement and
log_min_duration_statement, and not to statements included in other messages
(e.g., errors). Is that correct?

+ truncated_query = truncate_query_log(query_string);
+ query_log = truncated_query ? truncated_query : query_string;

In the patch, truncate_query_log() is called unconditionally in
exec_simple_query(), even when the query isn't logged. This adds unnecessary
overhead. It would be better to call it only when logging is actually performed
(e.g., under check_log_statement() or check_log_duration()). For example,

-------------------------------------------
/* Log immediately if dictated by log_statement */
if (check_log_statement(parsetree_list))
{
+ char *truncated_stmt = NULL;
+
+ if (log_statement_max_length >= 0)
+ truncated_stmt = truncate_query_log(query_string);
+
ereport(LOG,
- (errmsg("statement: %s", query_string),
+ (errmsg("statement: %s",
(truncated_stmt != NULL) ? truncated_stmt : query_string),
errhidestmt(true),
errdetail_execute(parsetree_list)));
was_logged = true;
+
+ if (truncated_stmt != NULL)
+ pfree(truncated_stmt);
}

<snip>

case 2:
- ereport(LOG,
- (errmsg("duration: %s ms
statement: %s",
- msec_str,
query_string),
- errhidestmt(true),
- errdetail_execute(parsetree_list)));
- break;
+ {
+ char *truncated_stmt = NULL;
+
+ if (log_statement_max_length >= 0)
+ truncated_stmt =
truncate_query_log(query_string);
+
+ ereport(LOG,
+ (errmsg("duration: %s
ms statement: %s",
+
msec_str, (truncated_stmt != NULL) ? truncated_stmt : query_string),
+ errhidestmt(true),
+
errdetail_execute(parsetree_list)));
+
+ if (truncated_stmt != NULL)
+ pfree(truncated_stmt);
+ break;
+ }
}
-------------------------------------------

+ char *truncated_query = NULL;
+ const char *query_log;

In exec_parse_message(), exec_bind_message(), and exec_execute_message(),
variables like truncated_query can be declared closer to where they are used
(e.g., inside the check_log_duration() switch case) to improve readability.
For example,

-------------------------------------------
break;
case 2:
- ereport(LOG,
- (errmsg("duration: %s ms
parse %s: %s",
- msec_str,
- *stmt_name ?
stmt_name : "<unnamed>",
- query_string),
- errhidestmt(true)));
- break;
+ {
+ char *truncated_stmt = NULL;
+
+ if (log_statement_max_length >= 0)
+ truncated_stmt =
truncate_query_log(query_string);
+
+ ereport(LOG,
+ (errmsg("duration: %s
ms parse %s: %s",
+ msec_str,
+
*stmt_name ? stmt_name : "<unnamed>",
+
(truncated_stmt != NULL) ? truncated_stmt : query_string),
+ errhidestmt(true)));
+
+ if (truncated_stmt != NULL)
+ pfree(truncated_stmt);
+ break;
+ }
}
-------------------------------------------

+ long_desc => '-1 means no truncation.',

+#log_statement_max_length = -1 # max length of logged statements
+ # -1 disables truncation

I like the description like "-1 means log statement in full", which seems
clearer and easier to understand for users. Thought?

Regarding the regression test, testing log_statement_max_length in pg_ctl test
looks a bit odd to me. It might be better to place it under
src/test/modules/test_misc, for example?

Regards,

--
Fujii Masao