Re: [PATCH] remove redundant ownership checks

(2010/01/12 10:27), Bruce Momjian wrote:
> Robert Haas wrote:
>> On Sun, Jan 10, 2010 at 4:54 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Robert Haas<robertmhaas(at)gmail(dot)com> writes:
>>>> I have looked this over a little bit and I guess I don't see why the
>>>> lack of a grand plan for how to organize all of our permissions checks
>>>> ought to keep us from removing this one on the grounds of redundancy.
>>>> We have to attack this problem in small pieces if we're going to make
>>>> any progress, and the pieces aren't going to get any smaller than
>>>> this.
>>>
>>> I would turn that argument around: given the lack of a grand plan,
>>> why should we remove this particular check at all? Nobody has argued
>>> that there would be a significant, or even measurable, performance gain.
>>> When and if we do have a plan, we might find ourselves putting this
>>> check back.
>>
>> You're arguing against a straw man - there's clearly no argument here
>> from performance. We generally do not choose to litter the code with
>> redundant or irrelevant checks because it makes the code difficult to
>> maintain and understand. Sometimes it also hurts performance, but
>> that's not a necessary criterion for removal. Nor are we generally in
>> the habit of keeping redundant code around because a hypothetical
>> future refactoring might by chance end up putting exactly the same
>> code back.
>
> I agree. Why are arbitrary restrictions being placed on code
> improvements? If code has no purpose, why not remove it, or at least
> mark it as NOT_USED.
>

The attached patch adds a source code comment which informs developers
that its own permission check had gone at the v8.5 release.

I also think we don't need to note it on the release-note. If we would
describe all the specification changes in external functions, is it
really valuable as a summary? It seems to me too details.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>