Re: pointless check in RelationBuildPartitionDesc

On 2018/09/04 13:08, Alvaro Herrera wrote:
> On 2018-Sep-04, Amit Langote wrote:
>
>> On 2018/09/04 10:19, Michael Paquier wrote:
>>> On Tue, Sep 04, 2018 at 09:47:07AM +0900, Amit Langote wrote:
>>>> On 2018/09/04 6:39, Alvaro Herrera wrote:
>>>>> After looking, it seems that this is just self-inflicted pain: for some
>>>>> reason, we store the pg_inherits row for a partition, and immediately
>>>>> afterwards compute and store its partition bound, which requires the
>>>>> above hack. But if we do things in the opposite order, this is no
>>>>> longer needed. I propose to remove it, as in the attached patch.
>>>>
>>>> +1. I remember having facepalmed at this before and had also written a
>>>> patch but never got around to submitting it.
>>>
>>> Ok, I see. It seems to me that this could be replaced by an
>>> elog(ERROR), as relispartition ought to be set anyway. This way any
>>> future callers would get things done in the correct order.
>>
>> Converting it to elog(ERROR, ...) might be a good idea.
>
> I think it'd be pointless noise. If we really want to protect against
> that, I think we should promote the Assert for relpartbound's isnull
> flag into an if test.

So that we can elog(ERROR, ...) if isnull is true? If so, I agree,
because that's what should've been done here anyway (for a value read from
the disk that is). I think we should check relispartition then too.

Thanks,
Amit