Re: Adjust errorcode in background worker code

On Sun, Jun 28, 2015 at 10:43 PM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2015-06-29 AM 11:36, Amit Langote wrote:
>> Hi,
>>
>> How about the attached that adjusts errorcode for the error related to
>> checking the flag bgw_flags in BackgroundWorkerInitializeConnection*()
>> functions so that it matches the treatment in SanityCheckBackgroundWorker()?
>>
>> s/ERRCODE_PROGRAM_LIMIT_EXCEEDED/ERRCODE_INVALID_PARAMETER_VALUE/g
>>
>> There is already a "/* XXX is this the right errcode? */" there.
>
> Oops, a wrong thing got attached.
>
> Please find correct one attached this time.

Well, I'm just catching up on some old email and saw this thread. I
like the idea of trying to use the best possible error code, but I'm
not so sure this is an improvement. One problem is that
ERRCODE_INVALID_PARAMETER_VALUE is that we use it, uh, a lot:

[rhaas pgsql]$ git grep ERRCODE_ | sed 's/.*ERRCODE_/ERRCODE_/;
s/[^A-Z0-9_].*//;' | sort | uniq -c | sort -n -r | head
540 ERRCODE_FEATURE_NOT_SUPPORTED
442 ERRCODE_INVALID_PARAMETER_VALUE
380 ERRCODE_SYNTAX_ERROR
194 ERRCODE_WRONG_OBJECT_TYPE
194 ERRCODE_UNDEFINED_OBJECT
181 ERRCODE_DATATYPE_MISMATCH
180 ERRCODE_INSUFFICIENT_PRIVILEGE
150 ERRCODE_INVALID_TEXT_REPRESENTATION
137 ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
123 ERRCODE_PROGRAM_LIMIT_EXCEEDED

I wonder if we need to think about inventing some new error codes. I
can sort of understand that "feature not supported" is something that
can come in a large number of different contexts and mean pretty much
the same all the time, but I'm betting that things like "invalid
parameter value" and "invalid text representation" and "object not in
prerequisite state" cover an amazing breadth of errors that may not
actually be that similar to each other.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company