Re: Reducing the log spam

On 11.03.25 10:46, Laurenz Albe wrote:
> Thanks for the thorough test!
>
>> There are a few issues though ...
>>
>> 1) Invalid codes aren't rejected. Is there any way to validate them?
>>
>> postgres=# SET log_suppress_errcodes TO '0foo1'; SHOW log_suppress_errcodes;
>> SET
>>  log_suppress_errcodes
>> -----------------------
>>  0foo1
>> (1 row)
> That is intentional. I only test that the SQLSTATE is 5 characters long
> and contains only ASCII letters and numbers. I looked at the SQL standard,
> and while it defines the meaning of certain SQLSTATEs, it allows for
> custom defined states.
>
> Nothing bad can happen from setting an unused SQLSTATE; it just won't
> suppress anything.

I suspected that. Just for the records, v5 now shows also "invalid"
SQLSTATEs in uppercase:

postgres=# SET log_suppress_errcodes TO 'x1y2z'; SHOW log_suppress_errcodes;
SET
 log_suppress_errcodes
-----------------------
 X1Y2Z
(1 row)

>> 2) No duplication check (not critical IMO)
>>
>> postgres=# SET log_suppress_errcodes TO '3F000,3F000'; SHOW
>> log_suppress_errcodes;
>> SET
>>  log_suppress_errcodes
>> -----------------------
>>  3F000,3F000
>> (1 row)
>>
>>
>> 3) errcodes are not trimmed (also not critical..  just doesn't look nice)
>>
>> postgres=# SET log_suppress_errcodes TO '               3F000, 42P01';
>> SHOW log_suppress_errcodes;
>> SET
>>     log_suppress_errcodes    
>> -----------------------------
>>                 3F000, 42P01
>> (1 row)
> These two are mostly cosmetic issues.
>
> I originally thought it best to leave the parameter the way the user
> entered it, but in the attached version I revised that by reassembling
> the parameter string from the parsed SQLSTATEs, so that spaces and
> duplicates will vanish and everything is converted to upper case.
>
> So these complaints should be addressed.

Duplicated entries and whitespaces are now removed

postgres=# SET log_suppress_errcodes TO '   3F000,         3F000, 42P01';
SHOW log_suppress_errcodes;
SET
 log_suppress_errcodes
-----------------------
 3F000,42P01
(1 row)

>
>> 4) SHOW log_suppress_errcodes displays an invalid value if we set it
>> twice to an empty string
>>
>> $ /usr/local/postgres-dev/bin/psql postgres
>> psql (18devel)
>> Type "help" for help.
>>
>> postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
>> SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
>> SET
>>  log_suppress_errcodes
>> -----------------------
>>  
>> (1 row)
>>
>> SET
>>  log_suppress_errcodes
>> -----------------------
>>  wV
>> (1 row)
>>
>>
>> 5) The system crashes if we set log_suppress_errcodesto an empty string
>> and then set it back to comma separated values
> These two points were actually caused by a memory management bug that I
> had inadvertently introduced in the assign hook. They should be fixed now.
>

empty log_suppress_errcode now behaves as expected

$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.

postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET
 log_suppress_errcodes
-----------------------
 
(1 row)

SET
 log_suppress_errcodes
-----------------------
 
(1 row)

The system no longer crashes when setting the parameter to an empty
string and set it back to comma separated values

$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.

postgres=# SET log_suppress_errcodes TO '3F000,42883'; SHOW
log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '42P01,23505'; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '3D000,42704'; SHOW log_suppress_errcodes;
SET
 log_suppress_errcodes
-----------------------
 3F000,42883
(1 row)

SET
 log_suppress_errcodes
-----------------------
 
(1 row)

SET
 log_suppress_errcodes
-----------------------
 42P01,23505
(1 row)

SET
 log_suppress_errcodes
-----------------------
 3D000,42704
(1 row)

The previous tests work as expected and all issues were addressed.

If the other reviewers have no objections, I'll mark this patch as
"Ready for Committer".

Best regards, Jim