Re: Truncate logs by max_log_size - Mailing list pgsql-hackers

On Sat, Apr 4, 2026 at 6:59 AM Jim Jones <jim.jones@uni-muenster.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