Обсуждение: log_error_verbosity function display
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. +
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
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. +
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 15LOCATION: parserOpenTable, parse_relation.c:858STATEMENT: 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 15LOCATION: parserOpenTable(), parse_relation.c:858STATEMENT: 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 15FUNCTION: parserOpenTable(), parse_relation.c:858STATEMENT: select * from lkjasdf; or SOURCE: ERROR: 42P01: relation "lkjasdf" does not exist at character 15SOURCE: parserOpenTable(), parse_relation.c:858STATEMENT: 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. +
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
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
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 15LOCATION: function parserOpenTable, file parse_relation.c:858STATEMENT: select * from lkjasdf; or even shorter: ERROR: 42P01: relation "lkjasdf" does not exist at character 15LOCATION: parserOpenTable() in file parse_relation.c:858STATEMENT: 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. +
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. +
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
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
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. +
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
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. +