On 2017/03/06 17:01, Ashutosh Bapat wrote:
> On Mon, Mar 6, 2017 at 1:26 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/03/06 16:49, Ashutosh Bapat wrote:
>>> On Mon, Mar 6, 2017 at 12:48 PM, Amit Langote wrote:
>>>> On 2017/03/06 15:41, Michael Paquier wrote:
>>>>> This comment is not completely correct. Children can be temp tables,
>>>>> they just cannot be temp tables of other backends. It seems to me that
>>>>> you could still keep this code simple and remove has_child..
>>>>
>>>> I updated the comment. I recall having posted a patch for that once, but
>>>> perhaps went unnoticed.
>>>
>>> The existing comment only specifies "temp tables" and not "temp table
>>> of other backends". The new comment keeps that part same and adds
>>> partitioned table case. So, I don't see any reason to change the "temp
>>> tables" to "temp table of other backends" in this patch.
>>
>> Hmm. A separate patch might be fine but why not fix the incorrect part
>> while we are updating the whole comment anyway.
>
> There must be a reason why that comment is written the way it's
> written. I guess, "from other backends" part of "temp tables" story
> has been explained just few lines above and the author/s didn't want
> to repeat it again.
That's perhaps true. I guess it depends on who reads it. Someone might
think the comment is incorrect because *not all* temporary child tables
are excluded.
> There's no point in changing it while we are not
> touching temp tables handling.
We can leave it for the committer to decide, maybe. Committers often
rewrite surrounding comments to improve wording, correcting factual
errors, etc.
Thanks,
Amit