From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: error messages in extended statistics |
Date: | 2019-05-04 23:42:33 |
Message-ID: | 20190504234233.l66mzyun4bykvdnq@development |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, May 03, 2019 at 09:42:17PM +0200, Tomas Vondra wrote:
>On Fri, May 03, 2019 at 12:21:36PM -0400, Tom Lane wrote:
>>Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
>>>Error reporting in extended statistics is inconsistent -- many messages
>>>that are ereport() in mvdistinct.c are elog() in the other modules.
>>>...
>>>I think this should be cleaned up, while at the same time not giving too
>>>much hassle for translators; for example, this message
>>>dependencies.c: elog(ERROR, "invalid MVDependencies size %zd (expected at least %zd)",
>>>should not only be turned into an ereport(), but also the MVDependencies
>>>part turned into a %s. (Alternatively, we could decide I was wrong and
>>>turn them all back into elogs, but I obviously vote against that.)
>>
>>FWIW, I'd vote the other way: that seems like a clear "internal error",
>>so making translators deal with it is just make-work. It should be an
>>elog. If there's a reasonably plausible way for a user to trigger an
>>error condition, then yes ereport, but if we're reporting situations
>>that couldn't happen without a server bug then elog seems fine.
>>
>
>Yeah, I agree. Most of (peshaps all) those errors are internal errors,
>and thus should be elog. I'll take care of clening this up a bit.
>
OK, so here is a patch, using elog() for all places except for the
input function, where we simply report we don't accept those values.
I agree those are internal errors, usually meaning the statistics object
was either corrupted or there's a bug in how it's built/serialized.
Users should not be able to trigger those cases (the only thing I can
think of is sending a bogus value through send/recv functions, that
simply do byteaout/byteain).
Now, what about backpatch? It's a small tweak, but it makes the life a
bit easier for translators ...
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
stats-log-cleanup.patch | text/plain | 2.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jonathan S. Katz | 2019-05-05 02:34:51 | Re: First-draft release notes for back branches are up |
Previous Message | Tomas Vondra | 2019-05-04 23:00:48 | Re: Logging the feature of SQL-level read/write commits |