Re: proposal: enable new error fields in plpgsql (9.4)

On Tue, Jun 25, 2013 at 03:56:27PM +0200, Pavel Stehule wrote:
> cleaned patch is in attachment

Of the five options you're adding to GET STACKED DIAGNOSTICS, four of them
appear in the SQL standard. DATATYPE_NAME does not; I think we should call it
PG_DATATYPE_NAME in line with our other extensions in this area.

> >> 2013/2/1 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> >> > 2013/2/1 Peter Eisentraut <peter_e(at)gmx(dot)net>:
> >> >> On 2/1/13 8:00 AM, Pavel Stehule wrote:
> >> >>> 2013/2/1 Marko Tiikkaja <pgmail(at)joh(dot)to>:
> >> >>>> Is there a compelling reason why we wouldn't provide these already in
> >> >>>> 9.3?
> >> >>>
> >> >>> a time for assign to last commitfest is out.
> >> >>>
> >> >>> this patch is relative simple and really close to enhanced error
> >> >>> fields feature - but depends if some from commiters will have a time
> >> >>> for commit to 9.3 - so I am expecting primary target 9.4, but I am not
> >> >>> be angry if it will be commited early.
> >> >>
> >> >> If we don't have access to those fields on PL/pgSQL, what was the point
> >> >> of the patch to begin with? Surely, accessing them from C wasn't the
> >> >> main use case?
> >> >>
> >> >
> >> > These fields are available for application developers now. But is a
> >> > true, so without this patch, GET STACKED DIAGNOSTICS statement will
> >> > not be fully consistent, because some fields are accessible and others
> >> > not

I am inclined to back-patch this to 9.3. The patch is fairly mechanical, and
I think the risk of introducing bugs is less than the risk that folks will be
confused by these new-in-9.3 error fields being accessible from libpq and the
protocol, yet inaccessible from PL/pgSQL.

The existing protocol documentation says things like this:

Table name: if the error was associated with a specific table, the
name of the table. (When this field is present, the schema name field
provides the name of the table's schema.)

The way you have defined RAISE does not enforce this; the user can set
TABLE_NAME without setting SCHEMA_NAME at all. I see a few options here:

1. Change RAISE to check the invariants thoroughly. For example, TABLE_NAME
would require SCHEMA name, and the pair would need to name an actual table.

2. Change RAISE to check the invariants simply. For example, it could check
that SCHEMA_NAME is present whenever TABLE_NAME is present but provide no
validation that the pair names an actual table. (I think the protocol
language basically allows this, though a brief note wouldn't hurt.)

3. Tweak the protocol documentation to clearly permit what the patch has RAISE
allow, namely the free-form use of these fields. This part of the protocol is
new in 9.3, so it won't be a big deal to change it. The libpq documentation
has similar verbiage to update.

I suppose I prefer #3. It seems fair for user code to employ these fields for
applications slightly broader than their core use, like a constraint name that
represents some userspace notion of a constraint rather than normal, cataloged
constraint. I can also imagine an application like replaying logs from
another server, recreating the messages as that server emitted them; #2 or #3
would suffice for that.

Looking beyond the immediate topic of PL/pgSQL, it seems fair for a FDW to use
these error fields to name remote objects not known locally. Suppose a
foreign INSERT fires a remote trigger, and that trigger violates a constraint
of some other remote table. Naming the remote objects would be a reasonable
design choice. postgres_fdw might have chosen to just copy fields from the
remote error (it does not do this today for the fields in question, though).
The FDW might not even have a notion of a schema, at which point it would
legitimately choose to leave that field unpopulated. Once we allow any part
of the system to generate such errors, we should let PL/pgSQL do the same.

Thoughts on that plan?

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com