On 03/07/2026 02:56, Fujii Masao wrote: > Thanks for the review! I've pushed the patch.
Awesome. Thanks! > While working on it, I found a few possible follow-up improvements. > > (1) > When log_statement is enabled, executing a prepared statement logs the > prepared query in a DETAIL message. For example: > > =# SET log_statement_max_length TO 10; > =# PREPARE test AS SELECT * FROM pgbench_accounts WHERE aid = $1; > LOG: statement: PREPARE te > =# EXECUTE test(1); > LOG: statement: EXECUTE te > DETAIL: prepare: PREPARE test AS SELECT * FROM pgbench_accounts > WHERE aid = $1; > > Should log_statement_max_length also apply to such query string in the > DETAIL message? > > (2) > When a bind parameter is truncated by > log_parameter_max_length, an ellipsis (...) is appended: > > =# SET log_parameter_max_length TO 5; > =# SELECT $1::text \bind 'abcdefghijk' \g > LOG: execute <unnamed>: SELECT $1::text > DETAIL: Parameters: $1 = 'abcde...' > > Would it make sense for log_statement_max_length to append an > ellipsis as well, so that users can easily tell when a statement has > been truncated? +1 Nice additions -- the feature gap is obvious, IMHO. Are you planning to work on it? I'm drowning in work right now and can only jump on it next week. > (3) > + query_len = strlen(query); > > truncate_query_log() uses strlen() only to determine whether the > query exceeds log_statement_max_length. Since the query can be very > large, would it be better to use > > strnlen(query, log_statement_max_length + MAX_MULTIBYTE_CHAR_LEN) > > instead, to avoid scanning the entire string? I'm not so sure about this one. At this point, isn't "query" already \0 terminated? I'm also wondering if it could affect pg_mbcliplen() down the road, since strnlen() can return a different value (log_statement_max_length + MAX_MULTIBYTE_CHAR_LEN) on large queries -- not tested yet. Thanks! Best, Jim
