| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Maxym Kharchenko <maxymkharchenko(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Kirill Gavrilov <diphantxm(at)gmail(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, Euler Taveira <euler(at)eulerto(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Truncate logs by max_log_size |
| Date: | 2026-07-08 17:02:40 |
| Message-ID: | 190ba1a8-69f4-4c8b-99cb-aa4691dece95@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Fujii
On 03.07.26 17:46, Fujii Masao wrote:
> On Fri, Jul 3, 2026 at 4:34 PM Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> wrote:
>> +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.
>
> I don't have plans to work on those at the moment, so please feel free
> to take them on if you have time!
>
>
>> 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.
>
> Yes, "query" should already be NUL-terminated here. The reason for
> using strnlen() is not to handle an unterminated string, but to avoid
> scanning the entire query when it's very large and we only need to
> know whether it exceeds log_statement_max_length.
>
> I think it's fine to pass the bounded length to pg_mbcliplen().
> It only needs enough input to find a multibyte-safe clipping point at
> or before log_statement_max_length, i.e., it doesn't need the full
> query length. The extra MAX_MULTIBYTE_CHAR_LEN bytes provide enough
> lookahead to handle a multibyte character boundary correctly.
>
> - query_len = strlen(query);
> + query_len = strnlen(query,
> + (size_t) log_statement_max_length + MAX_MULTIBYTE_CHAR_LEN);
Attached 0001 addressing the points you made:
* appending ellipsis to the query to indicate truncation (docs and tests
also updated accordingly). A side effect of this change is that setting
the parameter to 0 logs an ellipsis, which is not zero in length, but is
arguably correct, as it indicates that the whole query has been
truncated - just noting.
* use of strnlen to avoid scanning the whole query.
* truncation of prepared statements in DETAIL.
While working on this I noticed some redundant checks to call
truncate_query_log(), e.g. in exec_execute_message():
if (log_statement_max_length >= 0)
truncated_source = truncate_query_log(sourceText);
truncate_query_log() already returns NULL when query logging is disabled
or when no truncation is needed:
/* Truncation is disabled when the limit is negative */
if (!query || log_statement_max_length < 0)
return NULL;
So I'd argue that we don't need any checks in the caller.
- char *truncated_source = NULL;
-
- if (log_statement_max_length >= 0)
- truncated_source = truncate_query_log(sourceText);
+ char *truncated_source = truncate_query_log(sourceText);
0002 attached does this.
Thoughts?
BTW, should I open a new CF entry for this?
Best, Jim
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Improve-log_statement_max_length-truncation.patch | text/x-patch | 7.0 KB |
| v1-0002-Simplify-callers-of-truncate_query_log.patch | text/x-patch | 3.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Korotkov | 2026-07-08 17:03:48 | Re: SQL/JSON json_table plan clause |
| Previous Message | Rui Zhao | 2026-07-08 16:58:10 | Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement |