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

Thanks for the thorough review!

On 09/07/2026 06:03, Fujii Masao wrote:
> - errdetail("prepare: %s", pstmt->plansource->query_string);
> + char *truncated_stmt = truncate_query_log(pstmt->plansource->query_string);
> +
> + errdetail("prepare: %s", truncated_stmt ? truncated_stmt :
> pstmt->plansource->query_string);
> 
> Shouldn't truncated_stmt be freed with pfree() (when non-NULL)
> immediately after errdetail()?


Yes, I missed that one.
Fixed.


> -        is truncated to at most this many bytes.
> +        is truncated to at most this many bytes. When a statement is
> +        truncated, an ellipsis (<literal>...</literal>) is appended to
> +        indicate that truncation has occurred.
> 
> Since an ellipsis is appended, the logged output can exceed the configured
> limit by three bytes. Wouldn't it be clearer to say something like
> "has its statement text truncated to at most this many bytes"?
> That makes it clear the limit applies to the statement text, not the entire
> logged message.

Agreed.
I added your suggestion.

> -ok($node->log_contains(qr/statement: SELECT '123456789ABC$/m, $log_offset),
> +ok($node->log_contains(qr/statement: SELECT '123456789ABC...$/m, $log_offset),
> 
> Shouldn't the dots be escaped? As written, "..." matches any three characters.
> I think it should be "\.\.\.".


At first I also thought so, as in this context ... could mean any three
arbitrary characters, but the tests results showed a different picture,
so I thought it was not necessary. Anyway, I escaped them now in v2.


>     # a 24-byte query should end at the 20th byte ('C').
> 
> Since an ellipsis is now appended, isn't it better to replace this
> with "a 24-byte query should be clipped at the 20th byte ('C')
> and followed by an ellipsis."?
> 
> 
>     # With log_statement_max_length = 20, a 24-byte query should end
>     # at the 20th byte ('C').
> 
> Similarly, isn't it better to replace this with
> "With log_statement_max_length = 20, a 24-byte query should be clipped
> at the 20th byte ('C'). and followed by an ellipsis."?
>> * 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.
> So, should we update the docs and comments accordingly? For example:
> 
>  --------------------------------------------
> -        A value of zero causes statements to be logged with an empty body.
> +        A value of zero causes statements to be logged as only an ellipsis.
>          <literal>-1</literal> (the default) logs statements in full.
>          If this value is specified without units, it is taken as bytes.
> 
> { name => 'log_statement_max_length', type => 'int', context =>
> 'PGC_SUSET', group => 'LOGGING_WHAT',
> -  short_desc => 'Sets the maximum length in bytes of logged statements.',
> -  long_desc => '-1 means log statement in full; 0 means log an empty
> statement body.',
> +  short_desc => 'Sets the maximum length in bytes of logged statement text.',
> +  long_desc => '-1 means log statement in full; 0 means log only an ellipsis.',
> 
> - #log_statement_max_length = -1          # max bytes of logged statements;
> + #log_statement_max_length = -1          # max bytes of logged statement text;
>                                          # -1 means log statement in full,
> -                                        # 0 means log an empty body
> +                                        # 0 means log only an ellipsis
> 
> -# Verify 0 logs an empty statement body.
> +# Verify 0 logs only an ellipsis.
>  note "Zero length truncation";
> 
> ok($node->log_contains(qr/statement: \.\.\.\s*$/m, $log_offset),
> -       "0 logs statement body with ellipsis");
> +       "0 logs only an ellipsis");
>  --------------------------------------------


Comments and docs updated.


>> BTW, should I open a new CF entry for this?
> Yes, I think that's a good idea so these follow-up patches don't get forgotten.

Done: https://commitfest.postgresql.org/patch/6998/

Best, Jim