Re: [HACKERS] Oddity in error handling of constraint violation inExecConstraints for partitioned tables

Поиск
Список
Период
Сортировка
От Etsuro Fujita
Тема Re: [HACKERS] Oddity in error handling of constraint violation inExecConstraints for partitioned tables
Дата
Msg-id 56e0baa8-e458-2bbb-7936-367f7d832e43@lab.ntt.co.jp
обсуждение исходный текст
Ответ на Re: [HACKERS] Oddity in error handling of constraint violation inExecConstraints for partitioned tables  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Ответы Re: [HACKERS] Oddity in error handling of constraint violation inExecConstraints for partitioned tables  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Список pgsql-hackers
On 2017/07/07 18:47, Amit Langote wrote:
> On 2017/07/06 16:06, Etsuro Fujita wrote:
>> I think this should be fixed.  Attached is a patch for that.
> 
> Looking up the ResultRelInfo using the proposed method in
> ExecFindResultRelInfo() can be unreliable sometimes, that is, the method
> of using the root table OID to pin down the RangeTableEntry to get the the
> modified columns set.

You are right.  Thank you for pointing that out.

> How about setting ri_RangeTableIndex of the partition ResultRelInfo
> correctly in the first place?  If you look at the way InitResultRelInfo()
> is called in ExecSetupPartitionTupleRouting(), a dummy RT index of 1 is
> passed.  We could instead pass the correct one.  I think
> ModifyTable.nominalRelation is that (at least in the INSERT case.
> 
> The attached patch implements that.  It modifies
> ExecSetupPartitionTupleRouting to accept one more argument resultRTindex
> and passes along the same to InitResultRelInfo().  Later when
> ExecConstraints() wants to build the modifiedCols set, it looks up the
> correct RTE using the partition ResultRelInfo, which has the appropriate
> ri_RangeTableIndex set and uses the same.

Looks good to me.

> The patch keeps tests that you added in your patch.  Added this to the
> open items list.

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?

Best regards,
Etsuro Fujita

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Kyotaro HORIGUCHI
Дата:
Сообщение: Re: [HACKERS] hash index on unlogged tables doesn't behave asexpected
Следующее
От: Amit Langote
Дата:
Сообщение: Re: [HACKERS] New partitioning - some feedback