log_error_verbosity function display

Started by Bruce Momjianalmost 16 years ago13 messages
#1Bruce Momjian
bruce@momjian.us

Right now, log_error_verbosity displays the source code error location
in this format:

LOCATION: parserOpenTable, parse_relation.c:858

I think it would be clearer to add '()' next to the function name. We
use '() to display function names in our docs and I think using '()'
would clarify the output, e.g.:

LOCATION: parserOpenTable(), parse_relation.c:858

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

+ If your life is a hard drive, Christ can be your backup. +

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: log_error_verbosity function display

Bruce Momjian <bruce@momjian.us> writes:

Right now, log_error_verbosity displays the source code error location
in this format:

LOCATION: parserOpenTable, parse_relation.c:858

I think it would be clearer to add '()' next to the function name. We
use '() to display function names in our docs and I think using '()'
would clarify the output, e.g.:

LOCATION: parserOpenTable(), parse_relation.c:858

Seems like a waste of log space to me. The convention about writing ()
to decorate function names is hardly universal, and anyway it's mainly
useful to mark things that the reader might not realize are function
names. This can't be anything else.

regards, tom lane

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: log_error_verbosity function display

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Right now, log_error_verbosity displays the source code error location
in this format:

LOCATION: parserOpenTable, parse_relation.c:858

I think it would be clearer to add '()' next to the function name. We
use '() to display function names in our docs and I think using '()'
would clarify the output, e.g.:

LOCATION: parserOpenTable(), parse_relation.c:858

Seems like a waste of log space to me. The convention about writing ()
to decorate function names is hardly universal, and anyway it's mainly
useful to mark things that the reader might not realize are function
names. This can't be anything else.

I suggested it because it wasn't obvious to me it was a function name,
so I figured others might not recognize it. Remember, we deal with the
C code all the time so we have to consider how the general user would
see it.

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

+ If your life is a hard drive, Christ can be your backup. +

#4Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#3)
Re: log_error_verbosity function display

Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Right now, log_error_verbosity displays the source code error location
in this format:

LOCATION: parserOpenTable, parse_relation.c:858

I think it would be clearer to add '()' next to the function name. We
use '() to display function names in our docs and I think using '()'
would clarify the output, e.g.:

LOCATION: parserOpenTable(), parse_relation.c:858

Seems like a waste of log space to me. The convention about writing ()
to decorate function names is hardly universal, and anyway it's mainly
useful to mark things that the reader might not realize are function
names. This can't be anything else.

I suggested it because it wasn't obvious to me it was a function name,
so I figured others might not recognize it. Remember, we deal with the
C code all the time so we have to consider how the general user would
see it.

FYI, here is the output that had me confused:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable, parse_relation.c:858
STATEMENT: select * from lkjasdf;

Without the '()', I thought the LOCATION related to the query error
location, not the source code error location. This is what the new
format would look like, which I think is clearer:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable(), parse_relation.c:858
STATEMENT: select * from lkjasdf;

Of course, maybe the word LOCATION is wrong and it should be FUNCTION:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
FUNCTION: parserOpenTable(), parse_relation.c:858
STATEMENT: select * from lkjasdf;

or SOURCE:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
SOURCE: parserOpenTable(), parse_relation.c:858
STATEMENT: select * from lkjasdf;

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

+ If your life is a hard drive, Christ can be your backup. +

#5Alvaro Herrera
alvherre@commandprompt.com
In reply to: Bruce Momjian (#4)
Re: log_error_verbosity function display

Bruce Momjian wrote:

FYI, here is the output that had me confused:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable, parse_relation.c:858
STATEMENT: select * from lkjasdf;

How about something like

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable function in file parse_relation.c:858
STATEMENT: select * from lkjasdf;

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#6Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Bruce Momjian (#4)
Re: log_error_verbosity function display

On Thu, Feb 11, 2010 at 6:03 AM, Bruce Momjian <bruce@momjian.us> wrote:

Of course, maybe the word LOCATION is wrong and it should be FUNCTION:

       ERROR:  42P01: relation "lkjasdf" does not exist at character 15
       FUNCTION:  parserOpenTable(), parse_relation.c:858
       STATEMENT:  select * from lkjasdf;

i like this with or without the (), but maybe we are breaking client
apps if change that

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

#7Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#5)
Re: log_error_verbosity function display

Alvaro Herrera wrote:

Bruce Momjian wrote:

FYI, here is the output that had me confused:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable, parse_relation.c:858
STATEMENT: select * from lkjasdf;

How about something like

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable function in file parse_relation.c:858
STATEMENT: select * from lkjasdf;

Well, that looks good, but perhaps we can trim it down a bit:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: function parserOpenTable, file parse_relation.c:858
STATEMENT: select * from lkjasdf;

or even shorter:

ERROR: 42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable() in file parse_relation.c:858
STATEMENT: select * from lkjasdf;

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

+ If your life is a hard drive, Christ can be your backup. +

#8Bruce Momjian
bruce@momjian.us
In reply to: Jaime Casanova (#6)
Re: log_error_verbosity function display

Jaime Casanova wrote:

On Thu, Feb 11, 2010 at 6:03 AM, Bruce Momjian <bruce@momjian.us> wrote:

Of course, maybe the word LOCATION is wrong and it should be FUNCTION:

? ? ? ?ERROR: ?42P01: relation "lkjasdf" does not exist at character 15
? ? ? ?FUNCTION: ?parserOpenTable(), parse_relation.c:858
? ? ? ?STATEMENT: ?select * from lkjasdf;

i like this with or without the (), but maybe we are breaking client
apps if change that

Ah, so you like FUNCTION. We usually modify log information to improve
clarity. We aren't as strict in changes to log/admin stuff as we are
with SQL stuff.

Also, the exact content of log_error_verbosity wasn't even documented
until 9.0 with a pending patch I have.

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

+ If your life is a hard drive, Christ can be your backup. +

#9Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#5)
Re: log_error_verbosity function display

On Thu, Feb 11, 2010 at 2:08 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

Bruce Momjian wrote:

FYI, here is the output that had me confused:

      ERROR:  42P01: relation "lkjasdf" does not exist at character 15
      LOCATION:  parserOpenTable, parse_relation.c:858
      STATEMENT:  select * from lkjasdf;

How about something like

ERROR:  42P01: relation "lkjasdf" does not exist at character 15
LOCATION: parserOpenTable function in file parse_relation.c:858
STATEMENT:  select * from lkjasdf;

FWIW, I think I prefer Bruce's proposal to use FUNCTION as the tag,
rather than mentioning FUNCTION in the text. But I also wonder why
this bikeshed is not crimson.

...Robert

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#8)
Re: log_error_verbosity function display

Bruce Momjian <bruce@momjian.us> writes:

Jaime Casanova wrote:

i like this with or without the (), but maybe we are breaking client
apps if change that

Ah, so you like FUNCTION.

You can NOT change the line tag without almost certainly breaking
log-reading tools like pgfouine. Even changing the content of the line
risks that, and for no visible gain.

This seems like the worst form of bike-shedding to me. This log entry
has been formatted this way since 7.4, and nobody has ever complained
about it, until you suddenly decided it was a problem. Leave it be.

regards, tom lane

#11Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: log_error_verbosity function display

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Jaime Casanova wrote:

i like this with or without the (), but maybe we are breaking client
apps if change that

Ah, so you like FUNCTION.

You can NOT change the line tag without almost certainly breaking
log-reading tools like pgfouine. Even changing the content of the line
risks that, and for no visible gain.

This seems like the worst form of bike-shedding to me. This log entry
has been formatted this way since 7.4, and nobody has ever complained
about it, until you suddenly decided it was a problem. Leave it be.

I propose to add '()' because it is confusing without it. I don't think
many people are using the feature or we would have received suggestions
for improvmement. As you can see, once I posted about it, there were a
number of people who wanted improvements.

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

+ If your life is a hard drive, Christ can be your backup. +

#12Robert Haas
robertmhaas@gmail.com
In reply to: Bruce Momjian (#11)
Re: log_error_verbosity function display

On Thu, Feb 11, 2010 at 5:47 PM, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Jaime Casanova wrote:

i like this with or without the (), but maybe we are breaking client
apps if change that

Ah, so you like FUNCTION.

You can NOT change the line tag without almost certainly breaking
log-reading tools like pgfouine.  Even changing the content of the line
risks that, and for no visible gain.

This seems like the worst form of bike-shedding to me.  This log entry
has been formatted this way since 7.4, and nobody has ever complained
about it, until you suddenly decided it was a problem.  Leave it be.

I propose to add '()' because it is confusing without it.  I don't think
many people are using the feature or we would have received suggestions
for improvmement.  As you can see, once I posted about it, there were a
number of people who wanted improvements.

I'm not sure if people affirmatively wanted improvements or if people
were just discussing how to change it if a change was to be made. I
don't think you can infer that lack of suggestions for improvement
implies that no one is using it; it could equally well imply that
everyone likes it the way it is. To be sure, I probably would have
coded it a bit differently if I'd written the functionality
originally, but I don't think it's horrible the way it is, and Tom is
right that there is something to be said for consistency.

...Robert

#13Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#12)
Re: log_error_verbosity function display

Robert Haas wrote:

On Thu, Feb 11, 2010 at 5:47 PM, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Jaime Casanova wrote:

i like this with or without the (), but maybe we are breaking client
apps if change that

Ah, so you like FUNCTION.

You can NOT change the line tag without almost certainly breaking
log-reading tools like pgfouine. ?Even changing the content of the line
risks that, and for no visible gain.

This seems like the worst form of bike-shedding to me. ?This log entry
has been formatted this way since 7.4, and nobody has ever complained
about it, until you suddenly decided it was a problem. ?Leave it be.

I propose to add '()' because it is confusing without it. ?I don't think
many people are using the feature or we would have received suggestions
for improvmement. ?As you can see, once I posted about it, there were a
number of people who wanted improvements.

I'm not sure if people affirmatively wanted improvements or if people
were just discussing how to change it if a change was to be made. I
don't think you can infer that lack of suggestions for improvement
implies that no one is using it; it could equally well imply that
everyone likes it the way it is. To be sure, I probably would have
coded it a bit differently if I'd written the functionality
originally, but I don't think it's horrible the way it is, and Tom is
right that there is something to be said for consistency.

I have seen no other replies to this so I will not make any changes to
the output format.

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

+ If your life is a hard drive, Christ can be your backup. +