Little confusing things about client_min_messages.

Started by Tomonari Katsumataalmost 12 years ago7 messages
#1Tomonari Katsumata
t.katsumata1122@gmail.com
1 attachment(s)

Hi,

I noticed that the behavior of client_min_messages do not have
a consistency in document and 'pg_settings', postgresql.conf.

----------------
the behaviro is:
I can set 'INFO', 'FATAL' and 'PANIC' as the valid value.

postgres=# set client_min_messages to info;
SET
postgres=# set client_min_messages to fatal;
SET
postgres=# set client_min_messages to panic;
SET

----------------
document says:

<literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
<literal>WARNING</>, <literal>ERROR</>, <literal>FATAL</>,
and <literal>PANIC</>. Each level

I couldn't understand the reason of disappearing 'INFO' from the document.

----------------
pg_settings says:

{debug5,debug4,debug3,debug2,debug1,log,notice,warning,error}

and

postgresql.conf says:

#client_min_messages = notice # values in order of decreasing
detail:
# debug5
# debug4
# debug3
# debug2
# debug1
# log
# notice
# warning
# error

also I couldn't understand the reason of disappearing
'info', 'fatal' and 'panic' from them.
----------------

My proposal is all valid values should be present for users.
I fixed this, please see the attached patch.

regards,
-----------
Tomonari Katsumata

Attachments:

clear_client_min_messages.patchapplication/octet-stream; name=clear_client_min_messages.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2811f11..15ec9ef 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3795,9 +3795,9 @@ local0.*    /var/log/postgresql
         Controls which message levels are sent to the client.
         Valid values are <literal>DEBUG5</>,
         <literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
-        <literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
-        <literal>WARNING</>, <literal>ERROR</>, <literal>FATAL</>,
-        and <literal>PANIC</>.  Each level
+        <literal>DEBUG1</>, <literal>LOG</>, <literal>INFO</>,
+        <literal>NOTICE</>, <literal>WARNING</>, <literal>ERROR</>,
+        <literal>FATAL</>, and <literal>PANIC</>.  Each level
         includes all the levels that follow it.  The later the level,
         the fewer messages are sent.  The default is
         <literal>NOTICE</>.  Note that <literal>LOG</> has a different
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c76edb4..cad344b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -238,12 +238,12 @@ static const struct config_enum_entry client_message_level_options[] = {
 	{"debug2", DEBUG2, false},
 	{"debug1", DEBUG1, false},
 	{"log", LOG, false},
-	{"info", INFO, true},
+	{"info", INFO, false},
 	{"notice", NOTICE, false},
 	{"warning", WARNING, false},
 	{"error", ERROR, false},
-	{"fatal", FATAL, true},
-	{"panic", PANIC, true},
+	{"fatal", FATAL, false},
+	{"panic", PANIC, false},
 	{NULL, 0, false}
 };
 
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 3629a52..16ec760 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -360,9 +360,12 @@
 					#   debug2
 					#   debug1
 					#   log
+					#   info
 					#   notice
 					#   warning
 					#   error
+					#   fatal
+					#   panic
 
 #log_min_messages = warning		# values in order of decreasing detail:
 					#   debug5
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tomonari Katsumata (#1)
Re: Little confusing things about client_min_messages.

Tomonari Katsumata <t.katsumata1122@gmail.com> writes:

[ client_min_messages = info is not documented ]

That's intentional, because it's not a useful setting. Even more so
for the other two.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: Little confusing things about client_min_messages.

On Sat, Mar 8, 2014 at 11:31:22AM -0500, Tom Lane wrote:

Tomonari Katsumata <t.katsumata1122@gmail.com> writes:

[ client_min_messages = info is not documented ]

That's intentional, because it's not a useful setting. Even more so
for the other two.

Well, 'info' is between other settings we do document, so I am not clear
why info should be excluded. It is because we always output INFO to the
client? From elog.c:

if (ClientAuthInProgress)
output_to_client = (elevel >= ERROR);
else
output_to_client = (elevel >= client_min_messages ||
elevel == INFO);

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: Little confusing things about client_min_messages.

Bruce Momjian <bruce@momjian.us> writes:

On Sat, Mar 8, 2014 at 11:31:22AM -0500, Tom Lane wrote:

Tomonari Katsumata <t.katsumata1122@gmail.com> writes:

[ client_min_messages = info is not documented ]

That's intentional, because it's not a useful setting. Even more so
for the other two.

Well, 'info' is between other settings we do document, so I am not clear
why info should be excluded. It is because we always output INFO to the
client? From elog.c:

if (ClientAuthInProgress)
output_to_client = (elevel >= ERROR);
else
output_to_client = (elevel >= client_min_messages ||
elevel == INFO);

Right, so if you did set it to that, it wouldn't be functionally different
from NOTICE.

I'm not real sure why we allow setting client_min_messages to FATAL or
PANIC at all; seems to me that would break the FE/BE protocol, which says
that command cycles end with either the expected response or ErrorMessage.
In some quick experimentation, libpq/psql don't seem to get as confused as
I thought they would; but the user is sure likely to.

regression=# select 1/0;
ERROR: division by zero
regression=# set client_min_messages TO panic;
SET
regression=# select 1/0;
regression=# slect
regression-# ;
regression=# select 1.0;
?column?
----------
1.0
(1 row)

regression=# foo;
regression=#

So no, I don't think we ought to be advertising these as suggested
values. A saner proposed patch would be to remove them from the
valid values altogether. We probably had some good reason for leaving
them in the list back when, but I'm having a hard time reconstructing
what that would be.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Tomonari Katsumata
katsumata.tomonari@po.ntts.co.jp
In reply to: Tom Lane (#4)
1 attachment(s)
Re: Little confusing things about client_min_messages.

Hi Tom, Bruce,

Thank you for your response.

(2014/03/09 2:12), Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Sat, Mar 8, 2014 at 11:31:22AM -0500, Tom Lane wrote:

Tomonari Katsumata <t.katsumata1122@gmail.com> writes:

[ client_min_messages = info is not documented ]

That's intentional, because it's not a useful setting. Even more so
for the other two.

Well, 'info' is between other settings we do document, so I am not clear
why info should be excluded. It is because we always output INFO to the
client? From elog.c:

if (ClientAuthInProgress)
output_to_client = (elevel >= ERROR);
else
output_to_client = (elevel >= client_min_messages ||
elevel == INFO);

Right, so if you did set it to that, it wouldn't be functionally different
from NOTICE.

I understand it's intensional.
We can set INFO but it doesn't have any difference from NOTICE
because all INFO messages are sent to client.
So it is hidden from the document.

The word "valid" in the document has meant "meaningful".
I've misread it was "setable".

So no, I don't think we ought to be advertising these as suggested
values. A saner proposed patch would be to remove them from the
valid values altogether. We probably had some good reason for leaving
them in the list back when, but I'm having a hard time reconstructing
what that would be.

Adding FATAL and PANIC to client_min_messages is done at below-commit.
8ac386226d76b29a9f54c26b157e04e9b8368606
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606

According to the commit log, it seems that the purpose
is suppressing to be sent error message to client when "DROP TABLE".
In those days(pre 8.1), we did not have "DROP IF EXISTS" syntax,
so it was useful.

If this was the reason, now(from 8.2) we have "DROP IF EXISTS" syntax,
so we could delete FATAL and PANIC from client_min_messages valid value.
Attached patch do it. Please check it.

regards,
-----------
Tomonari Katsumata

Attachments:

clear_client_min_messages_v2.patchtext/x-diff; name=clear_client_min_messages_v2.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2811f11..cbaf264 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3796,8 +3796,7 @@ local0.*    /var/log/postgresql
         Valid values are <literal>DEBUG5</>,
         <literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
         <literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
-        <literal>WARNING</>, <literal>ERROR</>, <literal>FATAL</>,
-        and <literal>PANIC</>.  Each level
+        <literal>WARNING</>, and <literal>ERROR</>. Each level
         includes all the levels that follow it.  The later the level,
         the fewer messages are sent.  The default is
         <literal>NOTICE</>.  Note that <literal>LOG</> has a different
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c76edb4..b7cfe14 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -242,8 +242,6 @@ static const struct config_enum_entry client_message_level_options[] = {
 	{"notice", NOTICE, false},
 	{"warning", WARNING, false},
 	{"error", ERROR, false},
-	{"fatal", FATAL, true},
-	{"panic", PANIC, true},
 	{NULL, 0, false}
 };
 
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tomonari Katsumata (#5)
Re: Little confusing things about client_min_messages.

Tomonari Katsumata <katsumata.tomonari@po.ntts.co.jp> writes:

Adding FATAL and PANIC to client_min_messages is done at below-commit.
8ac386226d76b29a9f54c26b157e04e9b8368606
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606

According to the commit log, it seems that the purpose
is suppressing to be sent error message to client when "DROP TABLE".
In those days(pre 8.1), we did not have "DROP IF EXISTS" syntax,
so it was useful.

If this was the reason, now(from 8.2) we have "DROP IF EXISTS" syntax,

Uh, that was one example of what it might be good for; I doubt that the
use-case has now vanished entirely. While I'm still dubious about the
reliability of suppressing error messages, if people have been using this
type of coding for nearly 10 years then it probably works well enough
... and more to the point, they won't thank us for arbitrarily removing
it.

I think we should leave established practice alone here. It might be
confusing at first glance, but that doesn't mean it's the wrong thing.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Tomonari Katsumata
t.katsumata1122@gmail.com
In reply to: Tom Lane (#6)
Re: Little confusing things about client_min_messages.

Hi

2014-03-10 23:45 GMT+09:00 Tom Lane <tgl@sss.pgh.pa.us>:

Tomonari Katsumata <katsumata.tomonari@po.ntts.co.jp> writes:

Adding FATAL and PANIC to client_min_messages is done at below-commit.
8ac386226d76b29a9f54c26b157e04e9b8368606

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606

According to the commit log, it seems that the purpose
is suppressing to be sent error message to client when "DROP TABLE".
In those days(pre 8.1), we did not have "DROP IF EXISTS" syntax,
so it was useful.

If this was the reason, now(from 8.2) we have "DROP IF EXISTS" syntax,

Uh, that was one example of what it might be good for; I doubt that the
use-case has now vanished entirely. While I'm still dubious about the
reliability of suppressing error messages, if people have been using this
type of coding for nearly 10 years then it probably works well enough
... and more to the point, they won't thank us for arbitrarily removing
it.

Maybe so.

I think we should leave established practice alone here. It might be
confusing at first glance, but that doesn't mean it's the wrong thing.

I see.

If we delete it, it maybe become more confusing thing.

Thank you for your opinion.

regards,
---------------
Tomonari Katsumata