On Fri, 2025-03-07 at 20:38 +0100, Jim Jones wrote:
> I've tested this patch and for the most part it works as intended.
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.
> 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.
> 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.
Attached is the fifth version of the patch.
Yours,
Laurenz Albe
