elog() proposal

On Wed, Feb 20, 2002 at 09:54:51PM -0500, Bruce Momjian wrote:

I just submitted a patch to fix various elog() issues. I have two
additional proposals.

First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
compile problems/warnings, especially with Perl. I suggest renaming all
elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
E_* or E*. I am interested in other opinions.

You forgot PG_* :-)

Second, I propose adding two GUC variables that control how much elog()
info is sent to the server and client logs. I suggest
'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
FATAL, CRASH; and 'client_message_min' with possible values INFO,
NOTICE, ERROR, FATAL, CRASH.

IMHO stop an example NOTICE by "SET NOTICE TO 'OFF'" is better and
more common way than current the only one way in libpq and
PQsetNoticeProcessor.

Karel

--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/

C, PostgreSQL, PHP, WWW, http://docs.linux.cz, http://mape.jcu.cz

Bruce Momjian <pgman@candle.pha.pa.us> writes:

First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
compile problems/warnings, especially with Perl. I suggest renaming all
elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
E_* or E*. I am interested in other opinions.

We must put a prefix on these things. I have no strong opinion about
which prefix to use, but the change is long overdue.

Second, I propose adding two GUC variables that control how much elog()
info is sent to the server and client logs. I suggest
'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
FATAL, CRASH; and 'client_message_min' with possible values INFO,
NOTICE, ERROR, FATAL, CRASH.

client_message_min cannot usefully be set higher than ERROR. It will
certainly break libpq, for example, if 'E' messages don't come back when
a command fails. I also object to the idea that client_message_min
should not be settable to a level that allows DEBUG messages to be seen.
That would frequently save having to scrounge in the postmaster log,
which is not that easy if you're a client on a different machine.

We currently have 'debug_level' which controls how much debug
information is sent to the server logs. I believe this would have to
remain because it controls how _much_ DEBUG output is printed. We could
go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum
reporting mode with level 5.

I think you missed the point of my previous suggestion: let's invent
multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4,
and replace code like

if (DebugLevel > 4)
elog(DEBUG, ...);

with

elog(PGDEBUG4, ...);

This way, the log verbosity is controllable by a single mechanism rather
than two independent variables. You can set server_message_min to, say,
DEBUG or DEBUG4.

Finally, I still object strongly to the notion that any of these message
levels should be hard wired as "never send to client" or "never send to
log"; if you can't imagine scenarios where people will want to override
that behavior, then your imagination is lacking. I want that choice to
be completely configurable. What I'd suggest is an exclusion mask.
Suppose we order the levels as

PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR,
PGFATAL, PGCRASH.

Then I think from the client's point of view the only knob needed is
client_message_min (settable between PGDEBUG4 and PGERROR, with default
PGINFO). On the server side we could have server_message_min (settable
between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a
list server_message_exclude of levels not to log, which could have
default value "PGINFO,PGNOTICE" (internally this would become a bitmask
so it would be cheap to test).

It'd be possible to unify server_message_min and server_message_exclude
into a single parameter that's just a message level bitmask, but I think
the two separate parameters would be easier to deal with; otherwise you
end up with an awfully verbose parameter value.

regards, tom lane

Bruce Momjian writes:

First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
compile problems/warnings, especially with Perl. I suggest renaming all
elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
E_* or E*. I am interested in other opinions.

I think there should be separately named functions. That would relieve us
from inserting many dummy return statements because it gives the compiler
a clue of what's going on, and we don't have to tag all strings as
translatable.

Second, I propose adding two GUC variables that control how much elog()
info is sent to the server and client logs. I suggest
'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
FATAL, CRASH; and 'client_message_min' with possible values INFO,
NOTICE, ERROR, FATAL, CRASH.

I think there are a lot more possible combinations of behaviour we need to
take care of and we might as well get rid of this linear level system
altogether. Some categorizations:

* How to proceed: return to caller, abort transaction, abort session,
abort all sessions

* Where to send output: client, server log, both

* Type of error: code bug (assert), server failure, user/client error

Furthermore, in some instances you may want to send a different message to
client and server. (E.g., a client doesn't necessarily have the right to
know the exact cause of a server crash.)

Also, user errors need to be able to carry information such as error
codes.

So, what we need to do is to figure out exactly which of the above
combinations are useful or desirable, what parameters they take, etc. I
don't think just renaming a couple of things makes things better. Also,
there needs to be a transition plan, or suddenly every user-compiled
module is broken.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

I think there should be separately named functions.

You mean elog_debug(), elog_error(), ...? Seems pretty yucky.

That would relieve us
from inserting many dummy return statements because it gives the compiler
a clue of what's going on,

Only in gcc; if we remove the return statements then we still will get
warnings in compilers that don't recognize gcc's attributes. I think
this is a *bad* idea.

and we don't have to tag all strings as translatable.

Huh? How does having multiple functions instead of one help there?

I think there are a lot more possible combinations of behaviour we need to
take care of and we might as well get rid of this linear level system
altogether. Some categorizations:

* How to proceed: return to caller, abort transaction, abort session,
abort all sessions

* Where to send output: client, server log, both

* Type of error: code bug (assert), server failure, user/client error

Dubious. "How to proceed" clearly must be encoded in the elog call,
since generally the code surrounding the call point is not prepared to
cope with alternative behaviors. However, "where to send output" ought
to be configurable, as I have argued elsewhere. I'm not sure that "type
of error" really conveys much, or that we can always tell what the cause
of an error report is.

Furthermore, in some instances you may want to send a different message to
client and server.

Hmm, maybe, but I sure don't want to be passing two strings in every
elog call. There might be a *small* number of cases where the above is
true, and we could use a specialized variant of elog for them.

Also, user errors need to be able to carry information such as error
codes.

Agreed, but I suggested a grand revision of elog calls a year ago, and
no one had the energy to take it on. I think we shall have to resign
ourselves to fixing one issue at a time... unless you want to go back
to the grand plan?

there needs to be a transition plan, or suddenly every user-compiled
module is broken.

Also agreed. Probably the new function has to be called something else
than elog in any case, so that we can have a compatibility layer to
accept old elog calls. (That'd avoid requiring a "big bang" patch, too,
which'd surely ease making the changes.)

regards, tom lane

OK, let me throw out a modified proposal that attempts to deal with
these issues, and avoids creating a complicated error reporting system
that few will understand or be able to control.

How about if we have a new *_min_message setting, INFOLOG, which grabs
client and server log info. This will allow a unified system where the
client_min_message can be set to:

DEBUG, INFOLOG, INFO, NOTICE, ERROR

default, INFO, and the server_min_message can be:

DEBUG, INFOLOG, LOG, NOTICE, ERROR, FATAL, CRASH

default, LOG.

Basically we do this:

elog(INFO, ...)

and this prints to the client if its value is INFO or less, and
prints to the server if its level is INFOLOG or less. In this case:

elog(LOG, ...)

it prints to the client if its value is INFOLOG or less, and prints to
the server if its level is LOG or less.

This allows elog() to go to both places, if desired, and allows control
in a fine-grained manner.

This handles Tom's issue that DEBUG should be visible to the client if
it wishes.

Peter's issue of sending separate messages to client and server can be
handled with two elog messages, one LOG, another INFO.

This is all backward compatible because it does not remove any codes,
only adds elog levels INFO and LOG, which are lower than NOTICE.

Also, I like Tom's idea of DEBUG4 in the code, rather than testing the
debug level. Instead, do the level testing in elog() function, which
makes sense. This does allow DEBUGx as a possible debug level.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
compile problems/warnings, especially with Perl. I suggest renaming all
elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
E_* or E*. I am interested in other opinions.

We must put a prefix on these things. I have no strong opinion about
which prefix to use, but the change is long overdue.

Second, I propose adding two GUC variables that control how much elog()
info is sent to the server and client logs. I suggest
'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
FATAL, CRASH; and 'client_message_min' with possible values INFO,
NOTICE, ERROR, FATAL, CRASH.

client_message_min cannot usefully be set higher than ERROR. It will
certainly break libpq, for example, if 'E' messages don't come back when
a command fails. I also object to the idea that client_message_min
should not be settable to a level that allows DEBUG messages to be seen.
That would frequently save having to scrounge in the postmaster log,
which is not that easy if you're a client on a different machine.

We currently have 'debug_level' which controls how much debug
information is sent to the server logs. I believe this would have to
remain because it controls how _much_ DEBUG output is printed. We could
go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum
reporting mode with level 5.

I think you missed the point of my previous suggestion: let's invent
multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4,
and replace code like

if (DebugLevel > 4)
elog(DEBUG, ...);

with

elog(PGDEBUG4, ...);

This way, the log verbosity is controllable by a single mechanism rather
than two independent variables. You can set server_message_min to, say,
DEBUG or DEBUG4.

Finally, I still object strongly to the notion that any of these message
levels should be hard wired as "never send to client" or "never send to
log"; if you can't imagine scenarios where people will want to override
that behavior, then your imagination is lacking. I want that choice to
be completely configurable. What I'd suggest is an exclusion mask.
Suppose we order the levels as

PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR,
PGFATAL, PGCRASH.

Then I think from the client's point of view the only knob needed is
client_message_min (settable between PGDEBUG4 and PGERROR, with default
PGINFO). On the server side we could have server_message_min (settable
between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a
list server_message_exclude of levels not to log, which could have
default value "PGINFO,PGNOTICE" (internally this would become a bitmask
so it would be cheap to test).

It'd be possible to unify server_message_min and server_message_exclude
into a single parameter that's just a message level bitmask, but I think
the two separate parameters would be easier to deal with; otherwise you
end up with an awfully verbose parameter value.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Actually, it is even simpler. Let's do this:

Client levels:

DEBUG, LOG, INFO, NOTICE, ERROR

Server levels:

DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH

Where client LOG shows LOG, INFO and higher, and server INFO shows INFO,
LOG and higher.

Not sure if INFOLOG was clearer or not.

---------------------------------------------------------------------------

Bruce Momjian wrote:

OK, let me throw out a modified proposal that attempts to deal with
these issues, and avoids creating a complicated error reporting system
that few will understand or be able to control.

How about if we have a new *_min_message setting, INFOLOG, which grabs
client and server log info. This will allow a unified system where the
client_min_message can be set to:

DEBUG, INFOLOG, INFO, NOTICE, ERROR

default, INFO, and the server_min_message can be:

DEBUG, INFOLOG, LOG, NOTICE, ERROR, FATAL, CRASH

default, LOG.

Basically we do this:

elog(INFO, ...)

and this prints to the client if its value is INFO or less, and
prints to the server if its level is INFOLOG or less. In this case:

elog(LOG, ...)

it prints to the client if its value is INFOLOG or less, and prints to
the server if its level is LOG or less.

This allows elog() to go to both places, if desired, and allows control
in a fine-grained manner.

This handles Tom's issue that DEBUG should be visible to the client if
it wishes.

Peter's issue of sending separate messages to client and server can be
handled with two elog messages, one LOG, another INFO.

This is all backward compatible because it does not remove any codes,
only adds elog levels INFO and LOG, which are lower than NOTICE.

Also, I like Tom's idea of DEBUG4 in the code, rather than testing the
debug level. Instead, do the level testing in elog() function, which
makes sense. This does allow DEBUGx as a possible debug level.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
compile problems/warnings, especially with Perl. I suggest renaming all
elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
E_* or E*. I am interested in other opinions.

We must put a prefix on these things. I have no strong opinion about
which prefix to use, but the change is long overdue.

Second, I propose adding two GUC variables that control how much elog()
info is sent to the server and client logs. I suggest
'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
FATAL, CRASH; and 'client_message_min' with possible values INFO,
NOTICE, ERROR, FATAL, CRASH.

client_message_min cannot usefully be set higher than ERROR. It will
certainly break libpq, for example, if 'E' messages don't come back when
a command fails. I also object to the idea that client_message_min
should not be settable to a level that allows DEBUG messages to be seen.
That would frequently save having to scrounge in the postmaster log,
which is not that easy if you're a client on a different machine.

We currently have 'debug_level' which controls how much debug
information is sent to the server logs. I believe this would have to
remain because it controls how _much_ DEBUG output is printed. We could
go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum
reporting mode with level 5.

I think you missed the point of my previous suggestion: let's invent
multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4,
and replace code like

if (DebugLevel > 4)
elog(DEBUG, ...);

with

elog(PGDEBUG4, ...);

This way, the log verbosity is controllable by a single mechanism rather
than two independent variables. You can set server_message_min to, say,
DEBUG or DEBUG4.

Finally, I still object strongly to the notion that any of these message
levels should be hard wired as "never send to client" or "never send to
log"; if you can't imagine scenarios where people will want to override
that behavior, then your imagination is lacking. I want that choice to
be completely configurable. What I'd suggest is an exclusion mask.
Suppose we order the levels as

PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR,
PGFATAL, PGCRASH.

Then I think from the client's point of view the only knob needed is
client_message_min (settable between PGDEBUG4 and PGERROR, with default
PGINFO). On the server side we could have server_message_min (settable
between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a
list server_message_exclude of levels not to log, which could have
default value "PGINFO,PGNOTICE" (internally this would become a bitmask
so it would be cheap to test).

It'd be possible to unify server_message_min and server_message_exclude
into a single parameter that's just a message level bitmask, but I think
the two separate parameters would be easier to deal with; otherwise you
end up with an awfully verbose parameter value.

regards, tom lane

-- 
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 853-3000
+  If your life is a hard drive,     |  830 Blythe Avenue
+  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

OK, let me throw out a modified proposal that attempts to deal with
these issues, and avoids creating a complicated error reporting system
that few will understand or be able to control.

Question: does any of this help with the problem (I assume it's a problem)
that a syntax error in a statement causes the transaction to be rolled back?

Very annoying in psql...

Chris

Christopher Kings-Lynne wrote:

OK, let me throw out a modified proposal that attempts to deal with
these issues, and avoids creating a complicated error reporting system
that few will understand or be able to control.

Question: does any of this help with the problem (I assume it's a problem)
that a syntax error in a statement causes the transaction to be rolled back?

Very annoying in psql...

No, that is savepoints. I have a proposal on that in TODO.detail but
not sure if it is acceptable or when I can implement it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Actually, it is even simpler. Let's do this:
Client levels:
DEBUG, LOG, INFO, NOTICE, ERROR
Server levels:
DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH

Hmm, so the two cases have different ideas of the ordering of the
levels? Could be confusing, but it does keep the configuration
entries simple-looking.

What's your reaction to Peter's comments that the whole notion of
a linear set of elog levels should be abandoned?

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Actually, it is even simpler. Let's do this:
Client levels:
DEBUG, LOG, INFO, NOTICE, ERROR
Server levels:
DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH

Hmm, so the two cases have different ideas of the ordering of the
levels? Could be confusing, but it does keep the configuration
entries simple-looking.

What's your reaction to Peter's comments that the whole notion of
a linear set of elog levels should be abandoned?

I don't want to get into a second-system effect where we develop a
system that is hard to manage. We do need error codes, and I think this
system will fit into that when we decide to do it.

However, we would still need a system of reporting control if we went
with codes. I don't see a way around that. I have seen the linear
error systems where everything is numbers, and things that are 9X are
serious and -1X are not, but it seems quite confusing. Eventually we
can base codes on these levels we have defined and go from there.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tom Lane writes:

Peter Eisentraut <peter_e@gmx.net> writes:

I think there should be separately named functions.

You mean elog_debug(), elog_error(), ...? Seems pretty yucky.

I would rather avoid the name "elog" altogether, for semantic and
compatibility reasons. We could invent much nicer names, e.g.,

LogDebug(int, const char*, ...)
LogInfo(const char*, ...)
AbortTransaction(const char*, ...)
AbortSession(const char*, ...)
AbortAllSessions(const char*, ...)

and we don't have to tag all strings as translatable.

Huh? How does having multiple functions instead of one help there?

The string extractor can only go by function name, not arguments.

I'm not sure that "type of error" really conveys much, or that we can
always tell what the cause of an error report is.

What I mean with "type of error" is that there's a significant difference
between user errors and server-side errors:

1. User errors should not necessarily go into the server log, unless
command logging is enabled.

2. User errors will eventually carry additional information such as error
codes. Server errors will just get one default error code.

3. Users should not necessarily be allowed to see the details of server
errors at the client side, only some generic message.

So if we made up two separate functions each for errors and notices, we
could raise the awareness about this, even if initially the functionality
would not differ much.

Also agreed. Probably the new function has to be called something else
than elog in any case, so that we can have a compatibility layer to
accept old elog calls. (That'd avoid requiring a "big bang" patch, too,
which'd surely ease making the changes.)

Exactly.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

Tom Lane writes:

Peter Eisentraut <peter_e@gmx.net> writes:

I think there should be separately named functions.

You mean elog_debug(), elog_error(), ...? Seems pretty yucky.

I would rather avoid the name "elog" altogether, for semantic and
compatibility reasons. We could invent much nicer names, e.g.,

LogDebug(int, const char*, ...)
LogInfo(const char*, ...)
AbortTransaction(const char*, ...)
AbortSession(const char*, ...)
AbortAllSessions(const char*, ...)

Wow, seems awfully complex to me. Doesn't our current system seem
clearer:

elog(DEBUG, ...)
elog(NOTICE, ...)
elog(ERROR, ...)

Our elog messages are long enough already.

and we don't have to tag all strings as translatable.

Huh? How does having multiple functions instead of one help there?

The string extractor can only go by function name, not arguments.

Can't we hack it to pull out only certain elogs()? Also, don't we have
to translate everything? I guess not.

I'm not sure that "type of error" really conveys much, or that we can
always tell what the cause of an error report is.

What I mean with "type of error" is that there's a significant difference
between user errors and server-side errors:

1. User errors should not necessarily go into the server log, unless
command logging is enabled.

2. User errors will eventually carry additional information such as error
codes. Server errors will just get one default error code.

3. Users should not necessarily be allowed to see the details of server
errors at the client side, only some generic message.

So if we made up two separate functions each for errors and notices, we
could raise the awareness about this, even if initially the functionality
would not differ much.

Seems my solution is smaller and backward compatible. I don't see the
value in tons of options.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Peter Eisentraut <peter_e@gmx.net> writes:

You mean elog_debug(), elog_error(), ...? Seems pretty yucky.

I would rather avoid the name "elog" altogether, for semantic and
compatibility reasons.

Okay; it was never a particularly nice name anyway. But ---

We could invent much nicer names, e.g.,

LogDebug(int, const char*, ...)
LogInfo(const char*, ...)
AbortTransaction(const char*, ...)
AbortSession(const char*, ...)
AbortAllSessions(const char*, ...)

Well, (a) AbortTransaction is taken; (b) I would like to see these names
kept as short as possible, to leave more room on the line for the error
message text. We could do something like this involving abbreviations,
I suppose, but the existing and proposed mnemonics (LOG, INFO, ...,
CRASH) seem perfectly usable to me.

and we don't have to tag all strings as translatable.

Huh? How does having multiple functions instead of one help there?

The string extractor can only go by function name, not arguments.

Oh, now I get the point: you want to not pick up debug-level messages
for translation at all. That makes sense. Okay, how about we take
the names Bruce was proposing and make them function names:

PGLog(msg, ...)
PGError(msg, ...)
PGCrash(msg, ...)

regards, tom lane

Oh, now I get the point: you want to not pick up debug-level messages
for translation at all. That makes sense. Okay, how about we take
the names Bruce was proposing and make them function names:

PGLog(msg, ...)
PGError(msg, ...)
PGCrash(msg, ...)

OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
sense. Should we consider hiding these in macros so they really still
call elog(ERROR, ...) for backward compatiblity?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
sense. Should we consider hiding these in macros so they really still
call elog(ERROR, ...) for backward compatiblity?

I would love to make them macros, but I don't know a portable way to
create macros with variable numbers of arguments. Do you feel like
writing double parens?

PGERROR((msg, ...))

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
sense. Should we consider hiding these in macros so they really still
call elog(ERROR, ...) for backward compatiblity?

I would love to make them macros, but I don't know a portable way to
create macros with variable numbers of arguments. Do you feel like
writing double parens?

PGERROR((msg, ...))

Then we have to wonder what PGError is getting us that elog(ERROR)
isn't, except the ability to do internationalization based on the first
parameter.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian writes:

Can't we hack it to pull out only certain elogs()? Also, don't we have
to translate everything? I guess not.

I'm not sure. Someone other than me raised this point once. It's not so
important. I supposed, eventually people will want to translate
everything. Feel free to keep it as once function.

What I mean with "type of error" is that there's a significant difference
between user errors and server-side errors:

1. User errors should not necessarily go into the server log, unless
command logging is enabled.

2. User errors will eventually carry additional information such as error
codes. Server errors will just get one default error code.

3. Users should not necessarily be allowed to see the details of server
errors at the client side, only some generic message.

So if we made up two separate functions each for errors and notices, we
could raise the awareness about this, even if initially the functionality
would not differ much.

Seems my solution is smaller and backward compatible.

Your solution renumbers the error codes, so it's definitely not
backward-compatible.

I don't see the value in tons of options.

Well, I do. We don't need the separate user-side error functions
initially, but eventually we will have to have them.

So, basically, what this comes down to with respect to your patch:

1. Renumbering the error codes breaks backward compatibility *silently*.

2. CRASH doesn't seem like a good name to me.

3. I agree with adding a LOG or INFO level between DEBUG and NOTICE.

4. I don't like the alignment change. That seems very un-computer-like.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

Bruce Momjian writes:

Can't we hack it to pull out only certain elogs()? Also, don't we have
to translate everything? I guess not.

I'm not sure. Someone other than me raised this point once. It's not so
important. I supposed, eventually people will want to translate
everything. Feel free to keep it as once function.

OK. Glad it isn't a big issue.

What I mean with "type of error" is that there's a significant difference
between user errors and server-side errors:

1. User errors should not necessarily go into the server log, unless
command logging is enabled.

2. User errors will eventually carry additional information such as error
codes. Server errors will just get one default error code.

3. Users should not necessarily be allowed to see the details of server
errors at the client side, only some generic message.

So if we made up two separate functions each for errors and notices, we
could raise the awareness about this, even if initially the functionality
would not differ much.

Seems my solution is smaller and backward compatible.

Your solution renumbers the error codes, so it's definitely not
backward-compatible.

I don't need to renumber them. It is backward compatible at a source
code level, not an object code level. Is object code backward
compability for elog() an issue? If so, I don't need to renumber them.

I don't see the value in tons of options.

Well, I do. We don't need the separate user-side error functions
initially, but eventually we will have to have them.

So, basically, what this comes down to with respect to your patch:

1. Renumbering the error codes breaks backward compatibility *silently*.

Breaks object code only, which I think is minor, but I don't have to.

2. CRASH doesn't seem like a good name to me.

Tom and I came up with that one. Feel free to suggest another.

3. I agree with adding a LOG or INFO level between DEBUG and NOTICE.

Good.

4. I don't like the alignment change. That seems very un-computer-like.

So you want two spaces after every colon, no matter what? Sure. I just
makes the server logs jagged but it is a win on the user side.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Well, I do. We don't need the separate user-side error functions
initially, but eventually we will have to have them.

So, basically, what this comes down to with respect to your patch:

1. Renumbering the error codes breaks backward compatibility *silently*.

OK, so people want the old codes kept. I just have to warn people that
there will be some funky test structure in elog.c to handle message
*_min_messages tests. Is that OK?

2. CRASH doesn't seem like a good name to me.

OK, changed to FATALALL, which is pretty descriptive.

3. I agree with adding a LOG or INFO level between DEBUG and NOTICE.

OK, good, that was the crux of the patch. The rest was cleanups.

4. I don't like the alignment change. That seems very un-computer-like.

OK, removed, not always two spaces.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026