Subject: Re: Truncate logs by max_log_size

From: Jim Jones <[email-masked]>
Date: Thu, 28 Nov 2024 15:22:55 +0100

On 25.11.24 17:52, Kirill Reshke wrote:
> Hello! Please deliver the most recent patch version and fix the issues
> Jim identified [0] as the current commitfest draws to a close.
> Do not forget to include both parts of this patch (This was actually
> developed off-list, and we are now using this on our cloud PostgreSQL
> distribution on versions 12–17).
>
> [0] https://www.postgresql.org/message-id/35096a36-04d4-480b-a7cd-a2d8151fb737%40uni-muenster.de

In addition to these points, this feature seems to fail with queries
containing special characters (more than one byte):

With this "max_log_size"..

postgres=# SHOW max_log_size;
 max_log_size
--------------
 20
(1 row)

... and this query ..

postgres=# SELECT "ÄÜÖ" FROM t;
ERROR:  relation "t" does not exist
LINE 1: SELECT "ÄÜÖ" FROM t;

.. this is the [truncated] log entry we get ..

2024-11-28 14:58:57.912 CET [2258876] ERROR:  relation "t" does not
exist at character 19
2024-11-28 14:58:57.912 CET [2258876] STATEMENT:  SELECT "ÄÜÖ" FROM

... although the query originally had exactly 20 characters:

postgres=# SELECT length('SELECT "ÄÜÖ" FROM t;');
 length
--------
     20
(1 row)

postgres=# SELECT length('ÄÜÖ'::bytea), length('AUO'::bytea);
 length | length
--------+--------
      6 |      3
(1 row)

If it is supposed to be like this, it should be clearly stated so in the
docs.

-- 
Jim

---

From: Kirill Gavrilov <[email-masked]>
Date: Thu, 28 Nov 2024 22:20:42 +0300

On Thu, Nov 28, 2024 at 5:23 PM Jim Jones <[email-masked]> wrote:

> > [0]
> https://www.postgresql.org/message-id/35096a36-04d4-480b-a7cd-a2d8151fb737%40uni-muenster.de
>
>   Here is version 3 of this patch. I found another place where this
setting can be applied.
  Also added some documentation and specified that this setting truncates
queries by size in bytes.

---

From: Jim Jones <[email-masked]>
Date: Fri, 29 Nov 2024 01:24:24 +0100

On 28.11.24 20:20, Kirill Gavrilov wrote:
>   Also added some documentation and specified that this setting
> truncates queries by size in bytes.

Thanks. It is now possible to change the parameter using SET

postgres=# SET max_log_size TO 15;
SET
postgres=# SHOW max_log_size ;
 max_log_size
--------------
 15
(1 row)

In the postgresql.conf the default value is set to 0

#max_log_size = 0

But if we take a look at the parameter it shows something else

postgres=# SHOW max_log_size ;
 max_log_size
--------------
 5242880
(1 row)

Perhaps it would be best to keep the default value in the
postgresql.conf consistent with the documentation and the actual
max_log_size value. IMHO 0 (or -1) should disable it.

There are also several compilation issues:

postgres.c: In function ‘exec_simple_query’:
postgres.c:1040:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
 1040 |         bool copied = false;
      |         ^~~~
elog.c: In function ‘EmitErrorReport’:
elog.c:1697:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
 1697 |         const char* old_query_string = debug_query_string;
      |         ^~~~~
elog.c:1744:23: warning: passing argument 1 of ‘pfree’ discards ‘const’
qualifier from pointer target type [-Wdiscarded-qualifiers]
 1744 |                 pfree(debug_query_string);
      |                       ^~~~~~~~~~~~~~~~~~
In file included from ../../../../src/include/postgres.h:47,
                 from elog.c:55:
../../../../src/include/utils/palloc.h:86:25: note: expected ‘void *’
but argument is of type ‘const char *’
   86 | extern void pfree(void *pointer);
      |                   ~~~~~~^~~~~~~
elog.c: In function ‘build_query_log’:
elog.c:3798:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
 3798 |         size_t query_len = strlen(query);
      |         ^~~~~~
elog.c:3801:24: warning: return discards ‘const’ qualifier from pointer
target type [-Wdiscarded-qualifiers]
 3801 |                 return query;
      |                        ^~~~~
elog.c:3805:9: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
 3805 |         size_t query_log_len = pg_mbcliplen(query, query_len,
max_log_size);
      |         ^~~~~~

and there is a trailing whitespace at:

+ *    If query needs to be truncated, copied will be set to true

/home/jim/patches/max_log_query/V3-0001-parameter-max_log_size-to-truncate-logs.patch:141:
trailing whitespace.
 *      If query needs to be truncated, copied will be set to true
Checking patch doc/src/sgml/config.sgml...
Hunk #1 succeeded at 7865 (offset -48 lines).
Checking patch src/backend/tcop/postgres.c...
Hunk #1 succeeded at 71 (offset 1 line).
Hunk #2 succeeded at 1031 (offset 1 line).
Hunk #3 succeeded at 1083 (offset 1 line).
Hunk #4 succeeded at 1382 (offset 1 line).
Hunk #5 succeeded at 1393 (offset 1 line).
Checking patch src/backend/utils/error/elog.c...
Hunk #4 succeeded at 3781 (offset -3 lines).
Checking patch src/backend/utils/misc/guc_tables.c...
Hunk #1 succeeded at 3714 (offset -10 lines).
Checking patch src/backend/utils/misc/postgresql.conf.sample...
Hunk #1 succeeded at 615 (offset -2 lines).
Checking patch src/bin/pg_ctl/t/004_logrotate.pl...
Checking patch src/include/utils/elog.h...
Applied patch doc/src/sgml/config.sgml cleanly.
Applied patch src/backend/tcop/postgres.c cleanly.
Applied patch src/backend/utils/error/elog.c cleanly.
Applied patch src/backend/utils/misc/guc_tables.c cleanly.
Applied patch src/backend/utils/misc/postgresql.conf.sample cleanly.
Applied patch src/bin/pg_ctl/t/004_logrotate.pl cleanly.
Applied patch src/include/utils/elog.h cleanly.
warning: 1 line adds whitespace errors.

-- 
Jim

---

From: Kirill Gavrilov <[email-masked]>
Date: Fri, 29 Nov 2024 14:05:01 +0300

> postgres=# SHOW max_log_size ;
  Thanks for spotting mistakes.
  Setting max_log_size to 0 already disables truncation.
  I changed guc units to bytes, just to make it more human-friendly. Did
some refactoring and fixed all warnings. Set default value in
postgresql.conf.