Re: enhanced error fields

On 27 January 2013 18:57, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Geoghegan <peter.geoghegan86@gmail.com> writes:
>> I think we may be talking at cross purposes here. Guarantee may have
>> been too strong a word, or the wrong word entirely. All that I really
>> want here is for there to be a coding standard instituted, so that in
>> future client code will not be broken by a failure to include some
>> field in a new ereport site that related to what is effectively the
>> same error as an existing ereport site. I'm sure you'll agree that
>> subtly breaking client code when refactoring Postgres is unacceptable.
>
> [ shrug... ] If you have a way of making a guarantee that future
> versions introduce no new bugs, patent it and you'll soon have all the
> money in the world.

Is that kind of sarcasm really necessary?

Certain sets of ereport call sites within Postgres relate to either
very similar errors (i.e. ereports that have the same errcode) or
arguably identical errors (e.g. the various ERRCODE_CHECK_VIOLATION
sites within nbtinsert.c, that have identical error texts). It would
seem quite unfortunate to me if client code was to break based only on
an internal implementation detail that differed between Postgres
versions, or based on the current phase of the moon. This is the kind
of problem that I'd hoped to prevent by documenting a set of required
fields for a small number of errcodes going forward.

Now, you could take the view that all of this is only for the purposes
of error handling, and it isn't terribly critical that things work
very reliably. That isn't my view, though.

> It's conceivable that we could adapt some static checker to look for
> ereport calls that mention particular ERRCODEs and lack particular
> helper functions, but even that wouldn't be a cast-iron guarantee
> because of the possibility of call sites using non-constant errcode
> values.  It'd probably be good enough in practice though.

I thought about ways of doing that, but it didn't seem worth pursuing right now.

> However, this patch is not that, and mere documentation isn't going to buy a
> thing here IMO.  Especially not user-facing documentation, as opposed
> to something that might be in a developers' face when he's
> copying-and-pasting code somewhere.  This patch didn't even touch the
> one place in the documentation that might be somewhat useful from a
> developer's standpoint, which is 49.2. Reporting Errors Within the
> Server.

Well, an entry should probably be added to 49.2 too, then. Why should
documentation (of whatever kind deemed appropriate) not buy a thing?
Don't we want to prevent the kind of problems that I describe above?
How are people supposed to know about something that isn't written
down anywhere? Surely documentation is better than nothing?

> At some point we might want to undertake a round of refactoring that
> makes this type of information available; and once we do, we'd probably
> want to expose it in the regular message texts not just the auxiliary
> fields.  I think that sort of work is out-of-scope for this patch
> though.  IMO what we should be doing here is getting the infrastructure
> in place, and then decorating some basic set of messages with aux info
> in places where not a lot of new code is needed to make that happen.
> Extending the decoration beyond that is material for future work.

Fair enough.

-- 
Regards,
Peter Geoghegan