Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. plpgsql: make WHEN OTHERS distinct from WHEN SQLSTATE '00000'.

  1. Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-05T12:55:34Z

    Inspired by feedback to [1], I thought about how to reduce log spam.

My experience from the field is that a lot of log spam looks like

  database/table/... "xy" does not exist
  duplicate key value violates unique constraint "xy"

So what about a parameter "log_suppress_sqlstates" that suppresses
logging ERROR and FATAL messages with the enumerated SQL states?

My idea for a default setting would be something like

  log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'

but that's of course bikeshedding territory.

Yours,
Laurenz Albe



 [1]: https://postgr.es/m/b8b8502915e50f44deb111bc0b43a99e2733e117.camel%40cybertec.at

  2. Re: Reducing the log spam

    Pavel Stehule <pavel.stehule@gmail.com> — 2024-03-05T13:08:25Z

    Hi

út 5. 3. 2024 v 13:55 odesílatel Laurenz Albe <laurenz.albe@cybertec.at>
napsal:

> Inspired by feedback to [1], I thought about how to reduce log spam.
>
> My experience from the field is that a lot of log spam looks like
>
>   database/table/... "xy" does not exist
>   duplicate key value violates unique constraint "xy"
>
> So what about a parameter "log_suppress_sqlstates" that suppresses
> logging ERROR and FATAL messages with the enumerated SQL states?
>
> My idea for a default setting would be something like
>
>   log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'
>

+1 in this form

the overhead of this implementation should be small

Regards

Pavel


> but that's of course bikeshedding territory.
>
> Yours,
> Laurenz Albe
>
>
>
>  [1]:
> https://postgr.es/m/b8b8502915e50f44deb111bc0b43a99e2733e117.camel%40cybertec.at
>
>
>

  3. Re: Reducing the log spam

    Jim Jones <jim.jones@uni-muenster.de> — 2024-03-05T13:55:21Z

    Hi Laurenz

On 05.03.24 13:55, Laurenz Albe wrote:
> Inspired by feedback to [1], I thought about how to reduce log spam.
>
> My experience from the field is that a lot of log spam looks like
>
>    database/table/... "xy" does not exist
>    duplicate key value violates unique constraint "xy"
>
> So what about a parameter "log_suppress_sqlstates" that suppresses
> logging ERROR and FATAL messages with the enumerated SQL states?
>
> My idea for a default setting would be something like
>
>    log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'
>
> but that's of course bikeshedding territory.
>
> Yours,
> Laurenz Albe
>
>
>
>   [1]: https://postgr.es/m/b8b8502915e50f44deb111bc0b43a99e2733e117.camel%40cybertec.at

I like this idea, and I could see myself using it a lot in some projects.

Additionally, it would be nice to also have the possibility suppress a 
whole class instead of single SQL states, e.g.

log_suppress_sqlstates = 'class_08' to suppress these all at once:

08000 	connection_exception
08003 	connection_does_not_exist
08006 	connection_failure
08001 	sqlclient_unable_to_establish_sqlconnection
08004 	sqlserver_rejected_establishment_of_sqlconnection
08007 	transaction_resolution_unknown
08P01 	protocol_violation

Best regards,
Jim

  4. Re: Reducing the log spam

    Pavel Stehule <pavel.stehule@gmail.com> — 2024-03-05T13:57:05Z

    Hi

út 5. 3. 2024 v 14:55 odesílatel Jim Jones <jim.jones@uni-muenster.de>
napsal:

> Hi Laurenz
>
> On 05.03.24 13:55, Laurenz Albe wrote:
> > Inspired by feedback to [1], I thought about how to reduce log spam.
> >
> > My experience from the field is that a lot of log spam looks like
> >
> >    database/table/... "xy" does not exist
> >    duplicate key value violates unique constraint "xy"
> >
> > So what about a parameter "log_suppress_sqlstates" that suppresses
> > logging ERROR and FATAL messages with the enumerated SQL states?
> >
> > My idea for a default setting would be something like
> >
> >    log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'
> >
> > but that's of course bikeshedding territory.
> >
> > Yours,
> > Laurenz Albe
> >
> >
> >
> >   [1]:
> https://postgr.es/m/b8b8502915e50f44deb111bc0b43a99e2733e117.camel%40cybertec.at
>
> I like this idea, and I could see myself using it a lot in some projects.
>
> Additionally, it would be nice to also have the possibility suppress a
> whole class instead of single SQL states, e.g.
>
> log_suppress_sqlstates = 'class_08' to suppress these all at once:
>
> 08000   connection_exception
> 08003   connection_does_not_exist
> 08006   connection_failure
> 08001   sqlclient_unable_to_establish_sqlconnection
> 08004   sqlserver_rejected_establishment_of_sqlconnection
> 08007   transaction_resolution_unknown
> 08P01   protocol_violation
>
>
It can take code from PLpgSQL.

Regards

Pavel



> Best regards,
> Jim
>
>
>
>

  5. Re: Reducing the log spam

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-03-05T14:08:35Z

    On Tue, 5 Mar 2024 at 14:55, Jim Jones <jim.jones@uni-muenster.de> wrote:
> > So what about a parameter "log_suppress_sqlstates" that suppresses
> > logging ERROR and FATAL messages with the enumerated SQL states?

Big +1 from me for this idea.

  6. Re: Reducing the log spam

    Aleksander Alekseev <aleksander@timescale.com> — 2024-03-06T14:09:47Z

    Hi,

> So what about a parameter "log_suppress_sqlstates" that suppresses
> logging ERROR and FATAL messages with the enumerated SQL states?
>
> My idea for a default setting would be something like
>
>   log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'
>
> but that's of course bikeshedding territory.

I like the idea of suppressing certain log messages in general, but
the particular user interface doesn't strike me as an especially
convenient one.

Firstly I don't think many people remember sqlstates and what 3F000
stands for. IMO most users don't know such a thing exists. Secondly,
whether we should list sqlstates to suppress or the opposite - list
the states that shouldn't be suppressed, is a debatable question. Last
but not least, it's not quite clear whether PostgreSQL core is the
right place for implementing this functionality. For instance, one
could argue that the log message should just contain sqlstate and be
directed to |grep instead.

I suspect this could be one of "there is no one size fits all"
situations. The typical solution in such cases is to form a structure
containing the log message and its attributes and submit this
structure to a registered hook of a pluggable logging subsystem. This
would be the most flexible approach. It will allow not only filtering
the messages but also using binary logging, JSON logging, logging to
external systems like Loki instead of a given text file, etc.

I don't think we currently have this in the core, but maybe I just missed it.

-- 
Best regards,
Aleksander Alekseev

  7. Re: Reducing the log spam

    Greg Sabino Mullane <htamfids@gmail.com> — 2024-03-06T15:50:14Z

    On Tue, Mar 5, 2024 at 7:55 AM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:

> My experience from the field is that a lot of log spam looks like
>
>   database/table/... "xy" does not exist
>   duplicate key value violates unique constraint "xy"


Forcibly hiding those at the Postgres level seems a heavy hammer for what
is ultimately an application problem.

Tell me about a system that logs different classes of errors to different
log files, and I'm interested again.

Cheers,
Greg

  8. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-06T16:01:12Z

    On Wed, 2024-03-06 at 17:09 +0300, Aleksander Alekseev wrote:
> I like the idea of suppressing certain log messages in general, but
> the particular user interface doesn't strike me as an especially
> convenient one.
> 
> Firstly I don't think many people remember sqlstates and what 3F000
> stands for. IMO most users don't know such a thing exists. Secondly,
> whether we should list sqlstates to suppress or the opposite - list
> the states that shouldn't be suppressed, is a debatable question. Last
> but not least, it's not quite clear whether PostgreSQL core is the
> right place for implementing this functionality. For instance, one
> could argue that the log message should just contain sqlstate and be
> directed to |grep instead.
> 
> I suspect this could be one of "there is no one size fits all"
> situations. The typical solution in such cases is to form a structure
> containing the log message and its attributes and submit this
> structure to a registered hook of a pluggable logging subsystem. This
> would be the most flexible approach. It will allow not only filtering
> the messages but also using binary logging, JSON logging, logging to
> external systems like Loki instead of a given text file, etc.
> 
> I don't think we currently have this in the core, but maybe I just missed it.

The target would not primarily be installations where people configure
nifty logging software to filter logs (those people know how to deal
with log spam), but installations where people don't even know enough
to configure "shared_buffers".  So I'd like something that is part of
core and reduces spam without the user needing to configure anything.

I am somewhat worried that people will come up with all kinds of
justified but complicated wishes for such a feature:

- an option to choose whether to include or to exclude certain errors
- be able to configure that certain errors be logged on FATAL, but
  not on ERROR
- allow exception names in addition to SQL states
- have wildcards for exception names
- ...

I would like to write a simple patch that covers the basic functionality
I described, provided enough people find it useful.  That does not
exclude the option for future extensions for this feature.

Yours,
Laurenz Albe

  9. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-06T20:31:17Z

    On Wed, 2024-03-06 at 10:50 -0500, Greg Sabino Mullane wrote:
> On Tue, Mar 5, 2024 at 7:55 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> > My experience from the field is that a lot of log spam looks like
> > 
> >   database/table/... "xy" does not exist
> >   duplicate key value violates unique constraint "xy"
> 
> Forcibly hiding those at the Postgres level seems a heavy hammer for what is ultimately an application problem.

Yes... or no.  Lots of applications violate constraints routinely.
As long as the error is caught and handled, that's not a problem.

Whoever cares about the log messages can enable them.  My impression
is that most people don't care about them.

But thanks for your opinion.

Yours,
Laurenz Albe

  10. Re: Reducing the log spam

    Isaac Morland <isaac.morland@gmail.com> — 2024-03-06T22:33:33Z

    On Tue, 5 Mar 2024 at 07:55, Laurenz Albe <laurenz.albe@cybertec.at> wrote:

> Inspired by feedback to [1], I thought about how to reduce log spam.
>
> My experience from the field is that a lot of log spam looks like
>
>   database/table/... "xy" does not exist
>   duplicate key value violates unique constraint "xy"
>
> So what about a parameter "log_suppress_sqlstates" that suppresses
> logging ERROR and FATAL messages with the enumerated SQL states?
>
> My idea for a default setting would be something like
>
>   log_suppress_sqlstates = '23505,3D000,3F000,42601,42704,42883,42P01'
>
> but that's of course bikeshedding territory.
>

I like the basic idea and the way of specifying states seems likely to
cover a lot of typical use cases. Of course in principle the application
should be fixed, but in practice we can't always control that.

I have two questions about this:

First, can it be done per role? If I have a particular application which is
constantly throwing some particular error, I might want to suppress it, but
not suppress the same error occasionally coming from another application. I
see ALTER DATABASE name SET configuration_parameter … as being useful here,
but often multiple applications share a database.

Second, where can this setting be adjusted? Can any session turn off
logging of arbitrary sets of sqlstates resulting from its queries? It feels
to me like that might allow security problems to be hidden. Specifically,
the first thing an SQL injection might do would be to turn off logging of
important error states, then proceed to try various nefarious things.

It seems to me the above questions interact; an answer to the first might
be "ALTER ROLE role_specification SET configuration_parameter", but I think
that would allow roles to change their own settings, contrary to the
concern raised by the second question.

  11. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-07T07:30:59Z

    On Wed, 2024-03-06 at 17:33 -0500, Isaac Morland wrote:
> I have two questions about this:
> 
> First, can it be done per role? If I have a particular application which is
> constantly throwing some particular error, I might want to suppress it, but
> not suppress the same error occasionally coming from another application.
> I see ALTER DATABASE name SET configuration_parameter … as being useful here,
> but often multiple applications share a database.
>
> Second, where can this setting be adjusted? Can any session turn off logging
> of arbitrary sets of sqlstates resulting from its queries? It feels to me
> like that might allow security problems to be hidden. Specifically, the first
> thing an SQL injection might do would be to turn off logging of important
> error states, then proceed to try various nefarious things.

I was envisioning the parameter to be like other logging parameters, for
example "log_statement":  only superusers can set the parameter or GRANT
that privilege to others.  Also, a superuser could use ALTER ROLE to set
the parameter for all sessions by that role.

> It seems to me the above questions interact; an answer to the first might be
> "ALTER ROLE role_specification SET configuration_parameter", but I think that
> would allow roles to change their own settings, contrary to the concern
> raised by the second question.

If a superuser sets "log_statement" on a role, that role cannot undo or change
the setting.  That's just how I plan to implement the new parameter.

Yours,
Laurenz Albe

  12. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-09T13:03:55Z

    On Thu, 2024-03-07 at 08:30 +0100, Laurenz Albe wrote:
> On Wed, 2024-03-06 at 17:33 -0500, Isaac Morland wrote:
> > I have two questions about this:
> > 
> > First, can it be done per role? If I have a particular application which is
> > constantly throwing some particular error, I might want to suppress it, but
> > not suppress the same error occasionally coming from another application.
> > I see ALTER DATABASE name SET configuration_parameter … as being useful here,
> > but often multiple applications share a database.
> > 
> > Second, where can this setting be adjusted? Can any session turn off logging
> > of arbitrary sets of sqlstates resulting from its queries? It feels to me
> > like that might allow security problems to be hidden. Specifically, the first
> > thing an SQL injection might do would be to turn off logging of important
> > error states, then proceed to try various nefarious things.
> 
> I was envisioning the parameter to be like other logging parameters, for
> example "log_statement":  only superusers can set the parameter or GRANT
> that privilege to others.  Also, a superuser could use ALTER ROLE to set
> the parameter for all sessions by that role.
> 
> > It seems to me the above questions interact; an answer to the first might be
> > "ALTER ROLE role_specification SET configuration_parameter", but I think that
> > would allow roles to change their own settings, contrary to the concern
> > raised by the second question.
> 
> If a superuser sets "log_statement" on a role, that role cannot undo or change
> the setting.  That's just how I plan to implement the new parameter.

Here is a patch that implements this.

I went with "log_suppress_errcodes", since the term "error code" is used
throughout our documentation.

The initial value is 23505,3D000,3F000,42601,42704,42883,42P01,57P03

Yours,
Laurenz Albe

  13. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-11T02:43:58Z

    On Sat, 2024-03-09 at 14:03 +0100, Laurenz Albe wrote:
> Here is a patch that implements this.

And here is patch v2 that fixes a bug and passes the regression tests.

Yours,
Laurenz Albe

  14. Re: Reducing the log spam

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-03-11T08:33:48Z

    -   the subscriber's server log.
+   the subscriber's server log if you remove <literal>23505</literal> from
+   <xref linkend="guc-log-suppress-errcodes"/>.

This seems like a pretty big regression. Being able to know why your
replication got closed seems pretty critical.

On Mon, 11 Mar 2024 at 03:44, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> On Sat, 2024-03-09 at 14:03 +0100, Laurenz Albe wrote:
> > Here is a patch that implements this.
>
> And here is patch v2 that fixes a bug and passes the regression tests.
>
> Yours,
> Laurenz Albe

  15. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-03-11T11:18:01Z

    On Mon, 2024-03-11 at 09:33 +0100, Jelte Fennema-Nio wrote:
> -   the subscriber's server log.
> +   the subscriber's server log if you remove <literal>23505</literal> from
> +   <xref linkend="guc-log-suppress-errcodes"/>.
> 
> This seems like a pretty big regression. Being able to know why your
> replication got closed seems pretty critical.

The actual SQLSTATEs that get suppressed are subject to discussion
(an I have a gut feeling that some people will want the list empty).

As far as this specific functionality is concerned, I think that the
actual problem is a deficiency in PostgreSQL.  The problem is that
the log is the *only* place where you can get this information.  That
will be a problem for many people, even without "log_suppress_errcodes".

I think that this isformation should be available in some statistics
view.

Yours,
Laurenz Albe

  16. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-05-02T10:47:45Z

    On Mon, 2024-03-11 at 09:33 +0100, Jelte Fennema-Nio wrote:
> -   the subscriber's server log.
> +   the subscriber's server log if you remove <literal>23505</literal> from
> +   <xref linkend="guc-log-suppress-errcodes"/>.
> 
> This seems like a pretty big regression. Being able to know why your
> replication got closed seems pretty critical.

Yes.  But I'd argue that that is a shortcoming of logical replication:
there should be a ways to get this information via SQL.  Having to look into
the log file is not a very useful option.

The feature will become much less useful if unique voilations keep getting logged.

Yours,
Laurenz Albe

  17. Re: Reducing the log spam

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-05-02T11:08:30Z

    On Thu, 2 May 2024 at 12:47, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> Yes.  But I'd argue that that is a shortcoming of logical replication:
> there should be a ways to get this information via SQL.  Having to look into
> the log file is not a very useful option.

Definitely agreed that accessing the error details using SQL would be
much better. But having no way at all (by default) to find the cause
of the failure is clearly much worse.

> The feature will become much less useful if unique voilations keep getting logged.

Agreed. How about changing the patch so that the GUC is not applied to
logical replication apply workers (and document that accordingly). I
can think of two ways of achieving that (but there might be
other/better ones):
1. Set the GUC to empty string when an apply worker is started.
2. Change the newly added check in errcode() to only set
output_to_server to false when IsLogicalWorker() returns false.

  18. Re: Reducing the log spam

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-05-02T11:11:44Z

    On Thu, 2 May 2024 at 13:08, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> 2. Change the newly added check in errcode() to only set
> output_to_server to false when IsLogicalWorker() returns false.

Actually a third, and probably even better solution would be to only
apply this new GUC to non-backgroundworker processes. That seems quite
reasonable, since often the only way to access background worker
errors is often through the logs.

  19. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-05-03T12:49:38Z

    On Thu, 2024-05-02 at 13:11 +0200, Jelte Fennema-Nio wrote:
> On Thu, 2 May 2024 at 13:08, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> > 2. Change the newly added check in errcode() to only set
> > output_to_server to false when IsLogicalWorker() returns false.
> 
> Actually a third, and probably even better solution would be to only
> apply this new GUC to non-backgroundworker processes. That seems quite
> reasonable, since often the only way to access background worker
> errors is often through the logs.

That is a good idea.  This version only suppresses error messages
in ordinary backend processes.

Yours,
Laurenz Albe

  20. Re: Reducing the log spam

    Justin Pryzby <pryzby@telsasoft.com> — 2024-06-17T21:40:01Z

    On Thu, May 02, 2024 at 12:47:45PM +0200, Laurenz Albe wrote:
> On Mon, 2024-03-11 at 09:33 +0100, Jelte Fennema-Nio wrote:
> > -   the subscriber's server log.
> > +   the subscriber's server log if you remove <literal>23505</literal> from
> > +   <xref linkend="guc-log-suppress-errcodes"/>.
> > 
> > This seems like a pretty big regression. Being able to know why your
> > replication got closed seems pretty critical.
> 
> Yes.  But I'd argue that that is a shortcoming of logical replication:
> there should be a ways to get this information via SQL.  Having to look into
> the log file is not a very useful option.
> 
> The feature will become much less useful if unique voilations keep getting logged.

Uh, to be clear, your patch is changing the *defaults*, which I found
surprising, even after reaading the thread.  Evidently, the current
behavior is not what you want, and you want to change it, but I'm *sure*
that whatever default you want to use at your site/with your application
is going to make someone else unhappy.  I surely want unique violations
to be logged, for example.

> @@ -6892,6 +6892,41 @@ local0.*    /var/log/postgresql
>        </listitem>
>       </varlistentry>
>  
> +     <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
> +      <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
> +      <indexterm>
> +       <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
> +      </indexterm>
> +      </term>
> +      <listitem>
> +       <para>
> +        Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
> +        from client backend processes with certain error codes to be excluded
> +        from the log.
> +        The value is a comma-separated list of five-character error codes as
> +        listed in <xref linkend="errcodes-appendix"/>.  An error code that
> +        represents a class of errors (ends with three zeros) suppresses logging
> +        of all error codes within that class.  For example, the entry
> +        <literal>08000</literal> (<literal>connection_exception</literal>)
> +        would suppress an error with code <literal>08P01</literal>
> +        (<literal>protocol_violation</literal>).  The default setting is
> +        <literal>23505,3D000,3F000,42601,42704,42883,42P01,57P03</literal>.
> +        Only superusers and users with the appropriate <literal>SET</literal>
> +        privilege can change this setting.
> +       </para>

> +
> +       <para>
> +        This setting is useful to exclude error messages from the log that are
> +        frequent but irrelevant.

I think you should phrase the feature as ".. *allow* skipping error
logging for messages ... that are frequent but irrelevant for a given
site/role/DB/etc."

I suggest that this patch should not change the default behavior at all:
its default should be empty.  That you, personally, would plan to
exclude this or that error code is pretty uninteresting.  I think the
idea of changing the default behavior will kill the patch, and even if
you want to propose to do that, it should be a separate discussion.
Maybe you should make it an 002 patch.

> +		{"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
> +			gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
> +			NULL,
> +			GUC_LIST_INPUT
> +		},
> +		&log_suppress_errcodes,
> +		DEFAULT_LOG_SUPPRESS_ERRCODES,
> +		check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL

> +/*
> + * Default value for log_suppress_errcodes.  ERROR or FATAL messages with
> + * these error codes are never logged.  Error classes (error codes ending with
> + * three zeros) match all error codes in the class.   The idea is to suppress
> + * messages that usually don't indicate a serious problem but tend to pollute
> + * the log file.
> + */
> +
> +#define DEFAULT_LOG_SUPPRESS_ERRCODES "23505,3D000,3F000,42601,42704,42883,42P01,57P03"
> +

../src/backend/utils/errcodes.txt:23505    E    ERRCODE_UNIQUE_VIOLATION                                       unique_violation
../src/backend/utils/errcodes.txt:3D000    E    ERRCODE_INVALID_CATALOG_NAME                                   invalid_catalog_name
../src/backend/utils/errcodes.txt:3F000    E    ERRCODE_INVALID_SCHEMA_NAME                                    invalid_schema_name
../src/backend/utils/errcodes.txt:42601    E    ERRCODE_SYNTAX_ERROR                                           syntax_error
../src/backend/utils/errcodes.txt:3D000    E    ERRCODE_UNDEFINED_DATABASE
../src/backend/utils/errcodes.txt:42883    E    ERRCODE_UNDEFINED_FUNCTION                                     undefined_function
../src/backend/utils/errcodes.txt:3F000    E    ERRCODE_UNDEFINED_SCHEMA
../src/backend/utils/errcodes.txt:42P01    E    ERRCODE_UNDEFINED_TABLE                                        undefined_table
../src/backend/utils/errcodes.txt:42704    E    ERRCODE_UNDEFINED_OBJECT                                       undefined_object
../src/backend/utils/errcodes.txt:57P03    E    ERRCODE_CANNOT_CONNECT_NOW                                     cannot_connect_now

-- 
Justin

  21. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-06-18T16:49:36Z

    On Mon, 2024-06-17 at 16:40 -0500, Justin Pryzby wrote:
> > The feature will become much less useful if unique voilations keep getting logged.
> 
> Uh, to be clear, your patch is changing the *defaults*, which I found
> surprising, even after reaading the thread.  Evidently, the current
> behavior is not what you want, and you want to change it, but I'm *sure*
> that whatever default you want to use at your site/with your application
> is going to make someone else unhappy.  I surely want unique violations
> to be logged, for example.

I was afraid that setting the default non-empty would cause objections.

> > +       <para>
> > +        This setting is useful to exclude error messages from the log that are
> > +        frequent but irrelevant.
> 
> I think you should phrase the feature as ".. *allow* skipping error
> logging for messages ... that are frequent but irrelevant for a given
> site/role/DB/etc."

I have reworded that part.

> I suggest that this patch should not change the default behavior at all:
> its default should be empty.  That you, personally, would plan to
> exclude this or that error code is pretty uninteresting.  I think the
> idea of changing the default behavior will kill the patch, and even if
> you want to propose to do that, it should be a separate discussion.
> Maybe you should make it an 002 patch.

I have attached a new version that leaves the parameter empty by default.

The patch is not motivated by my personal dislike of certain error messages.
A well-written application would not need that parameter at all.
The motivation for me is based on my dealing with customers' log files,
which are often full of messages that are only distracting from serious
problems and fill up the disk.

But you are probably right that it would be hard to find a default setting
that nobody has quibbles with, and the default can always be changed with
a future patch.

Yours,
Laurenz Albe

  22. Re: Reducing the log spam

    Rafia Sabih <rafia.pghackers@gmail.com> — 2024-07-24T13:27:15Z

    Hello Laurenz,

I liked the idea for this patch. I will also go for the default being
an empty string.
I went through this patch and have some comments on the code,

1. In general, I don't like the idea of goto, maybe we can have a
free_something function to call here.

2.
if (!SplitIdentifierString(new_copy, ',', &states))
{
GUC_check_errdetail("List syntax is invalid.");
goto failed;
}
Here, we don't need all that free-ing, we can just return false here.

3.
/*
* Check the the values are alphanumeric and convert them to upper case
* (SplitIdentifierString converted them to lower case).
*/
for (p = state; *p != '\0'; p++)
if (*p >= 'a' && *p <= 'z')
*p += 'A' - 'a';
else if (*p < '0' || *p > '9')
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
I was thinking, maybe we can use tolower() function here.

4.
list_free(states);
pfree(new_copy);

*extra = statelist;
return true;

failed:
list_free(states);
pfree(new_copy);
guc_free(statelist);
return false;

This looks like duplication that can be easily avoided.
You may have free_somthing function to do all free-ing stuff only and
its callee can then have a return statement.
e.g for here,
free_states(states, new_copy, statelist);
return true;

5. Also, for alphanumeric check, maybe we can have something like,
if (isalnum(*state) == 0)
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
and we can do this in the beginning after the len check.

On Tue, 18 Jun 2024 at 18:49, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> On Mon, 2024-06-17 at 16:40 -0500, Justin Pryzby wrote:
> > > The feature will become much less useful if unique voilations keep getting logged.
> >
> > Uh, to be clear, your patch is changing the *defaults*, which I found
> > surprising, even after reaading the thread.  Evidently, the current
> > behavior is not what you want, and you want to change it, but I'm *sure*
> > that whatever default you want to use at your site/with your application
> > is going to make someone else unhappy.  I surely want unique violations
> > to be logged, for example.
>
> I was afraid that setting the default non-empty would cause objections.
>
> > > +       <para>
> > > +        This setting is useful to exclude error messages from the log that are
> > > +        frequent but irrelevant.
> >
> > I think you should phrase the feature as ".. *allow* skipping error
> > logging for messages ... that are frequent but irrelevant for a given
> > site/role/DB/etc."
>
> I have reworded that part.
>
> > I suggest that this patch should not change the default behavior at all:
> > its default should be empty.  That you, personally, would plan to
> > exclude this or that error code is pretty uninteresting.  I think the
> > idea of changing the default behavior will kill the patch, and even if
> > you want to propose to do that, it should be a separate discussion.
> > Maybe you should make it an 002 patch.
>
> I have attached a new version that leaves the parameter empty by default.
>
> The patch is not motivated by my personal dislike of certain error messages.
> A well-written application would not need that parameter at all.
> The motivation for me is based on my dealing with customers' log files,
> which are often full of messages that are only distracting from serious
> problems and fill up the disk.
>
> But you are probably right that it would be hard to find a default setting
> that nobody has quibbles with, and the default can always be changed with
> a future patch.
>
> Yours,
> Laurenz Albe



-- 
Regards,
Rafia Sabih

  23. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2024-07-25T16:03:23Z

    Thanks for the review!

On Wed, 2024-07-24 at 15:27 +0200, Rafia Sabih wrote:
> I liked the idea for this patch. I will also go for the default being
> an empty string.
> I went through this patch and have some comments on the code,
> 
> 1. In general, I don't like the idea of goto, maybe we can have a
> free_something function to call here.

The PostgreSQL code base has over 3000 goto's...

Sure, that can be factored out to a function (except the final "return"),
but I feel that a function for three "free" calls is code bloat.

Do you think that avoiding the goto and using a function would make the
code simpler and clearer?

> 2.
> if (!SplitIdentifierString(new_copy, ',', &states))
> {
> GUC_check_errdetail("List syntax is invalid.");
> goto failed;
> }
> Here, we don't need all that free-ing, we can just return false here.

I am OK with changing that; I had thought it was more clearer and more
foolproof to use the same pattern everywhere.

> 3.
> /*
> * Check the the values are alphanumeric and convert them to upper case
> * (SplitIdentifierString converted them to lower case).
> */
> for (p = state; *p != '\0'; p++)
> if (*p >= 'a' && *p <= 'z')
> *p += 'A' - 'a';
> else if (*p < '0' || *p > '9')
> {
> GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
> goto failed;
> }
> I was thinking, maybe we can use tolower() function here.

That is a good idea, but these C library respect the current locale.
I would have to explicitly specify the C locale or switch the locale
temporarily.

Switching the locale seems clumsy, and I have no idea what I would have
to feed as second argument to toupper_l() or isalnum_l().
Do you have an idea?

> 4.
> list_free(states);
> pfree(new_copy);
> 
> *extra = statelist;
> return true;
> 
> failed:
> list_free(states);
> pfree(new_copy);
> guc_free(statelist);
> return false;
> 
> This looks like duplication that can be easily avoided.
> You may have free_somthing function to do all free-ing stuff only and
> its callee can then have a return statement.
> e.g for here,
> free_states(states, new_copy, statelist);
> return true;

That free_states() function would just contain two function calls.
I think that defining a special function for that is somewhat out of
proportion.

> 5. Also, for alphanumeric check, maybe we can have something like,
> if (isalnum(*state) == 0)
> {
> GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
> goto failed;
> }
> and we can do this in the beginning after the len check.

isalnum() operates on a single character and depends on the current locale.
See my comments to 3. above.


Please let me know what you think, particularly about the locale problem.

Yours,
Laurenz Albe

  24. Re: Reducing the log spam

    Rafia Sabih <rafia.pghackers@gmail.com> — 2024-08-15T17:52:55Z

    On Thu, 25 Jul 2024 at 18:03, Laurenz Albe <laurenz.albe@cybertec.at> wrote:

> Thanks for the review!
>
> On Wed, 2024-07-24 at 15:27 +0200, Rafia Sabih wrote:
> > I liked the idea for this patch. I will also go for the default being
> > an empty string.
> > I went through this patch and have some comments on the code,
> >
> > 1. In general, I don't like the idea of goto, maybe we can have a
> > free_something function to call here.
>
> The PostgreSQL code base has over 3000 goto's...
>
> Sure, that can be factored out to a function (except the final "return"),
> but I feel that a function for three "free" calls is code bloat.
>
> On a detailed look over this, you are right Laurenz about this.

> Do you think that avoiding the goto and using a function would make the
> code simpler and clearer?
>
> > 2.
> > if (!SplitIdentifierString(new_copy, ',', &states))
> > {
> > GUC_check_errdetail("List syntax is invalid.");
> > goto failed;
> > }
> > Here, we don't need all that free-ing, we can just return false here.
>
> I am OK with changing that; I had thought it was more clearer and more
> foolproof to use the same pattern everywhere.
>
> > 3.
> > /*
> > * Check the the values are alphanumeric and convert them to upper case
> > * (SplitIdentifierString converted them to lower case).
> > */
> > for (p = state; *p != '\0'; p++)
> > if (*p >= 'a' && *p <= 'z')
> > *p += 'A' - 'a';
> > else if (*p < '0' || *p > '9')
> > {
> > GUC_check_errdetail("error codes can only contain digits and ASCII
> letters.");
> > goto failed;
> > }
> > I was thinking, maybe we can use tolower() function here.
>
> That is a good idea, but these C library respect the current locale.
> I would have to explicitly specify the C locale or switch the locale
> temporarily.
>
Hmm. actually I don't have any good answers to this locale issue.

>
> Switching the locale seems clumsy, and I have no idea what I would have
> to feed as second argument to toupper_l() or isalnum_l().
> Do you have an idea?
>
> > 4.
> > list_free(states);
> > pfree(new_copy);
> >
> > *extra = statelist;
> > return true;
> >
> > failed:
> > list_free(states);
> > pfree(new_copy);
> > guc_free(statelist);
> > return false;
> >
> > This looks like duplication that can be easily avoided.
> > You may have free_somthing function to do all free-ing stuff only and
> > its callee can then have a return statement.
> > e.g for here,
> > free_states(states, new_copy, statelist);
> > return true;
>
> That free_states() function would just contain two function calls.
> I think that defining a special function for that is somewhat out of
> proportion.
>
> > 5. Also, for alphanumeric check, maybe we can have something like,
> > if (isalnum(*state) == 0)
> > {
> > GUC_check_errdetail("error codes can only contain digits and ASCII
> letters.");
> > goto failed;
> > }
> > and we can do this in the beginning after the len check.
>
> isalnum() operates on a single character and depends on the current locale.
> See my comments to 3. above.
>
>
> Please let me know what you think, particularly about the locale problem.
>
> Yours,
> Laurenz Albe
>


-- 
Regards,
Rafia Sabih

  25. Re: Reducing the log spam

    Jim Jones <jim.jones@uni-muenster.de> — 2025-03-07T19:38:39Z

    Hi Laurenz

On 18.06.24 18:49, Laurenz Albe wrote:
> I have attached a new version that leaves the parameter empty by default.


I've tested this patch and for the most part it works as intended. For
convenience, I wrote a small function to simulate the exceptions using a
given errcode:

CREATE OR REPLACE FUNCTION raise_sqlstate(state text)
RETURNS VOID AS $$
BEGIN
  RAISE EXCEPTION 'exception sqlstate = %',
    state USING ERRCODE = state;
END;
$$ LANGUAGE plpgsql;

== specific errcodes ==

SET log_suppress_errcodes TO '23505,3D000,42601';
SHOW log_suppress_errcodes;

SELECT raise_sqlstate('23505'); -- must be suppressed
SELECT raise_sqlstate('3D000'); -- must be suppressed
SELECT raise_sqlstate('42601'); -- must be suppressed
SELECT raise_sqlstate('42P01');

log entries:
2025-03-07 14:23:30.688 CET [3266008] ERROR:  exception sqlstate = 42P01
2025-03-07 14:23:30.688 CET [3266008] CONTEXT:  PL/pgSQL function
raise_sqlstate(text) line 3 at RAISE
2025-03-07 14:23:30.688 CET [3266008] STATEMENT:  SELECT
raise_sqlstate('42P01');


== errcode classes ==

SET log_suppress_errcodes TO '23000,3D000';
SHOW log_suppress_errcodes;

SELECT raise_sqlstate('23505'); -- must be suppressed
SELECT raise_sqlstate('42P01');


log entries:
2025-03-07 14:24:40.023 CET [3268343] ERROR:  exception sqlstate = 42P01
2025-03-07 14:24:40.023 CET [3268343] CONTEXT:  PL/pgSQL function
raise_sqlstate(text) line 3 at RAISE
2025-03-07 14:24:40.023 CET [3268343] STATEMENT:  SELECT
raise_sqlstate('42P01');



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)


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)

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

$ /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)

server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
You are currently not connected to a database.

!?>


Best regards, Jim

  26. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2025-03-11T09:46:26Z

    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

  27. Re: Reducing the log spam

    Jim Jones <jim.jones@uni-muenster.de> — 2025-03-11T11:32:09Z

    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

  28. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2025-03-14T05:58:22Z

    On Tue, 2025-03-11 at 10:46 +0100, Laurenz Albe wrote:
> Attached is the fifth version of the patch.

... and here, for good measure, a pgindented version
Apart from that, no changes.

Yours,
Laurenz Albe

  29. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2025-03-15T06:12:22Z

    On Fri, 2025-03-14 at 06:58 +0100, Laurenz Albe wrote:
> On Tue, 2025-03-11 at 10:46 +0100, Laurenz Albe wrote:
> > Attached is the fifth version of the patch.
> 
> ... and here, for good measure, a pgindented version
> Apart from that, no changes.

... and here is v7, improved in the spirit of
https://postgr.es/m/E132D362-A669-4606-AFE1-B45C9DFCC141%40yesql.se

Yours,
Laurenz Albe

  30. Re: Reducing the log spam

    Jim Jones <jim.jones@uni-muenster.de> — 2025-03-17T08:12:50Z

    On 15.03.25 07:12, Laurenz Albe wrote:
> ... and here is v7, improved in the spirit of
> https://postgr.es/m/E132D362-A669-4606-AFE1-B45C9DFCC141%40yesql.se


I just revisited this patch and v7 passes all tests from [1,2].
LGTM.

Best regards, Jim

[1] -
https://www.postgresql.org/message-id/205505aa-4ba1-4d5b-b5f2-d006f14f4775%40uni-muenster.de
[2] -
https://www.postgresql.org/message-id/ec933afb-c8a3-4954-bb58-8f86b272a6f9%40uni-muenster.de

  31. Re: Reducing the log spam

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-04-02T13:11:37Z

    
On 2025/03/17 17:12, Jim Jones wrote:
> 
> On 15.03.25 07:12, Laurenz Albe wrote:
>> ... and here is v7, improved in the spirit of
>> https://postgr.es/m/E132D362-A669-4606-AFE1-B45C9DFCC141%40yesql.se
> 
> 
> I just revisited this patch and v7 passes all tests from [1,2].
> LGTM.

Since this patch is marked as ready for committer, I've started looking at it.

Filtering log messages by SQLSTATE might be useful for some users,
but I'm unsure if it should belong in the core. There are also other
potential filtering needs, such as by application name, client host,
database, or roles. Adding only SQLSTATE filtering may not be good idea,
while supporting all possible cases in the core wouldn't be practical either.

Given that, I think implementing this as an extension using emit_log_hook
would be a better approach. Anyway, I'd like to hear more opinions from
other hackers on this.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  32. Re: Reducing the log spam

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-02T19:23:38Z

    Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> Filtering log messages by SQLSTATE might be useful for some users,
> but I'm unsure if it should belong in the core. There are also other
> potential filtering needs, such as by application name, client host,
> database, or roles. Adding only SQLSTATE filtering may not be good idea,
> while supporting all possible cases in the core wouldn't be practical either.

> Given that, I think implementing this as an extension using emit_log_hook
> would be a better approach. Anyway, I'd like to hear more opinions from
> other hackers on this.

I took a brief look and I concur with Fujii-san's conclusion: this'd
be a fine extension a/k/a contrib module, but it seems a bit too
opinionated about what sort of filtering is needed to be appropriate
within elog.c itself.

Also, just as a minor coding thought, I'd suggest using -1 not 0
to terminate the array of encoded SQLSTATEs, rather than assuming
that nobody would write 00000.  Compare commit 58fdca220.

			regards, tom lane

  33. Re: Reducing the log spam

    Laurenz Albe <laurenz.albe@cybertec.at> — 2025-04-03T07:22:04Z

    On Wed, 2025-04-02 at 15:23 -0400, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> > Filtering log messages by SQLSTATE might be useful for some users,
> > but I'm unsure if it should belong in the core. There are also other
> > potential filtering needs, such as by application name, client host,
> > database, or roles. Adding only SQLSTATE filtering may not be good idea,
> > while supporting all possible cases in the core wouldn't be practical either.
> 
> > Given that, I think implementing this as an extension using emit_log_hook
> > would be a better approach. Anyway, I'd like to hear more opinions from
> > other hackers on this.
> 
> I took a brief look and I concur with Fujii-san's conclusion: this'd
> be a fine extension a/k/a contrib module, but it seems a bit too
> opinionated about what sort of filtering is needed to be appropriate
> within elog.c itself.
> 
> Also, just as a minor coding thought, I'd suggest using -1 not 0
> to terminate the array of encoded SQLSTATEs, rather than assuming
> that nobody would write 00000.  Compare commit 58fdca220.

I agree with these assessments and will withdraw the patch.

Yours,
Laurenz Albe