Re: tuple-routing and constraint violation error message, revisited

On Mon, Apr 10, 2017 at 8:14 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> On Fri, Mar 31, 2017 at 7:43 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> Summary is: We decided in f1b4c771ea7 [1] that passing the original slot
>> (one containing the tuple formatted per root partitioned table's tupdesc)
>> to ExecConstraints(), but that breaks certain cases. Imagine what would
>> happen if a BR insert trigger changed the tuple - the original slot would
>> not contain those changes. So, it seems better to convert (if necessary)
>> the tuple formatted per partition tupdesc after tuple-routing back to the
>> root table's format and use the converted tuple to make val_desc shown in
>> the message if an error occurs.
>>
>> Attached rebased version of the patch that I had originally proposed
>> (summary above is the commit message). Robert thought it would be fine to
>> show the row formatted per partition rowtype, but would look better if we
>> could show the column names as well (remember that we're trying to account
>> for possible differences in the ordering of columns between the root table
>> and leaf partitions to which tuples are routed.)
>>
>> Added this to PostgreSQL 10 open items list.
>
> The changes look good to me. Now, ExecConstraint() has three blocks
> which are almost similar, differing only in the constraints checked
> and the error message. It was manageable without partitioning and may
> be it's still manageable, but it's certainly being pushed to the
> limits. May be we should refactor error reporting code into a separate
> function and call it in those three places.

Yeah, possibly.

Thanks for the review. I have committed the patch.

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