Re: Oddity in error handling of constraint violation in ExecConstraints for partitioned tables

On 24 July 2017 at 12:11, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hi Amit,
>
> On 2017/07/24 14:09, Amit Khandekar wrote:
>>>> On 2017/07/10 14:15, Etsuro Fujita wrote:
>>>> Another thing I noticed is the error handling in ExecWithCheckOptions; it
>>>> doesn't take any care for partition tables, so the column description in
>>>> the error message for WCO_VIEW_CHECK is built using the partition table's
>>>> rowtype, not the root table's. But I think it'd be better to build that
>>>> using the root table's rowtype, like ExecConstraints, because that would
>>>> make the column description easier to understand since the parent view(s)
>>>> (from which WithCheckOptions evaluated there are created) would have been
>>>> defined on the root table. This seems independent from the above issue,
>>>> so I created a separate patch, which I'm attaching. What do you think
>>>> about that?
>>
>> + if (map != NULL)
>> + {
>> + tuple = do_convert_tuple(tuple, map);
>> + ExecStoreTuple(tuple, slot, InvalidBuffer, false);
>> + }
>>
>> Above, the tuple descriptor also needs to be set, since the parent and
>> child partitions can have different column ordering.
>>
>> Due to this, the following testcase crashes :
>>
>> CREATE TABLE range_parted (a text,b int, c int) partition by range (b);
>> CREATE VIEW upview AS SELECT * FROM range_parted WHERE (select c >
>> 120) WITH CHECK OPTION;
>> create table part_a_1_a_10(b int, c int, a text);
>> alter table range_parted attach partition part_a_1_a_10 for values
>> from (1) to (10);
>> insert into upview values ('a', 2, 100);
>
> Good catch. Thanks for creating the patch.
>
> There are other places with similar code viz. those in ExecConstraints()
> that would need fixing.

Ah ok. I should have noticed that. Thanks.

> Attached is the updated version of your patch.
Now that this is done, any particular reason it is not done in
ExecPartitionCheck() ? I see that there is a do_convert_tuple() in
that function, again without changing the slot descriptor.

--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company