Re: enhanced error fields

Peter Geoghegan <peter(at)2ndquadrant(dot)com> writes:
> [ eelog6.patch ]
> So, with the question of what fields to include and whether constraint
> name needs to be unambiguously resolvable addressed (I think), it
> appears that I've brought this one as far as I can. I'd still like to
> get input from a Perl hacker, but I think a committer needs to pick
> this up now.

I started to look this patch over. I think we can get to something
committable from here, but I'm having a problem with the concept that
we're going to "guarantee" anything about which additional fields might
be available for any given SQLSTATE. This is already quite broken for
the ERRCODE_NOT_NULL_VIOLATION case, and it's not hard to envision that
there will be other inconsistencies in future, even without the issue
that third-party code might use these SQLSTATEs without having gotten
the memo about additional fields.

If we were doing this from scratch we could perhaps fix that by using,
eg, two different SQLSTATEs for the column-not-null and
something-else-not-null cases. But it's probably too late to change the
SQLSTATEs for existing error cases; applications might be depending on
those. Anyway our choice of SQLSTATE is often constrained by the
standard.

I'm inclined to remove the "requirements" business altogether and just
document that these fields may be supplied, or words to that effect.
In practice, an application is going to know whether the particular
error case it's concerned about has the auxiliary fields, I should think.

> We also go to extra lengths to get a table_name for certain
> domain-related ereport sites.

A lot of that looks pretty broken to me, eg the changes in
ExecEvalCoerceToDomain are just hokum. (Even if the expression is
executing in a statement that has a result table, there's no very good
reason to think that the error has anything to do with the result
table.) It's possible we could restructure things so that coercions
performed as part of an assignment to a specific table column are
processed differently from other coercions, and have knowledge available
about what the target table/column is. But it would be a pretty
invasive change for limited benefit.

BTW, one thing that struck me in a quick look-through is that the
ERRCODE_FOREIGN_KEY_VIOLATION patches seem to inconsistently send
either the PK or FK rel as the "errtable". Is this really per spec?
I'd have sort of expected that the reported table ought to be the one
that the constraint belongs to, namely the FK table.

regards, tom lane