Re: A bug in mapping attributes in ATExecAttachPartition()

On 2017/06/15 18:05, Ashutosh Bapat wrote:
> On Thu, Jun 15, 2017 at 2:30 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/06/15 17:53, Ashutosh Bapat wrote:
>>> On Thu, Jun 15, 2017 at 2:12 PM, Amit Langote wrote:
>>>>> Both of the above comments are not related to the bug that is being fixed, but
>>>>> they apply to the same code where the bug exists. So instead of fixing it
>>>>> twice, may be we should expand the scope of this work to cover other
>>>>> refactoring needed in this area. That might save us some rebasing and commits.
>>>>
>>>> Are you saying that the patch posted on that thread should be brought over
>>>> and discussed here?
>>>
>>> Not the whole patch, but that one particular comment, which applies to
>>> the existing code in ATExecAttachPartition(). If we fix the existing
>>> code in ATExecAttachPartition(), the refactoring patch there will
>>> inherit it when rebased.
>>
>> Yes, I too meant only the refactoring patch, which I see as patch 0001 in
>> the series of patches that Jeevan posted with the following message:
>>
>> https://www.postgresql.org/message-id/CAOgcT0NeR%3D%2BTMRTw6oq_5WrJF%2B_xG91k_nGUub29Lnv5-qmQHw%40mail.gmail.com
> 
> I think we don't need to move that patch over to here, unless you see
> that some of that refactoring is useful here. I think, we should
> continue this thread and patch independent of what happens there. If
> and when this patch gets committed, that patch will need to be
> refactored.

I do see it as useful refactoring and a way to implement a feature (which
is perhaps something worth including into v10?)  It's just that the patch
I have posted here fixes bugs, which it would be nice to get committed first.

Anyway, I tried to implement the refactoring in patch 0002, which is not
all of the patch 0001 that Jeevan posted.  Please take a look.  I wondered
if we should emit a NOTICE when an individual leaf partition validation
can be skipped?  No point in adding a new test if the answer to that is
no, I'd think.

Attaching here 0001 which fixes the bug (unchanged from the previous
version) and 0002 which implements the refactoring (and the feature to
look at the individual leaf partitions' constraints to see if validation
can be skipped.)

Thanks,
Amit