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

Hello

2013/6/28 Noah Misch <noah(at)leadboat(dot)com>:
> 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.
>

yes, but It should be fixed in 9.3 enhanced fields too - it should be
consistent with PostgreSQL fields

>
>> >> 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.
>

+1

>
> 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.
>

I like #3 too. These fields should be used in custom code freely - and
I don't would create some limits. Developer can use it for application
code how he likes. It was designed for this purpose.

> 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.

+1

Regards

Pavel

>
>
> Thoughts on that plan?
>
> --
> Noah Misch
> EnterpriseDB http://www.enterprisedb.com