Re: [PATCH] remove redundant ownership checks

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.

> Even if you are right in your unsubstantiated hypothesis that this
> change will be a subset of any future change that is made with some plan
> in mind, I don't see that incremental revisions of the permissions check
> placement are a good way to approach the problem.  What I fear will
> result from that is gaps in permissions checking, depending on what
> combination of revisions of core and third-party code happen to get used
> in a given installation.
>
> I think we need a plan first, not random patches first.

I think the issue at hand is completely severable from the issue of a
more general plan. Again, when we find that we find that the code
does the same work twice, that's typically something we would want to
fix, independent of what else might or might not come later. That
having been said, I'm 100% in favor of talking about what a more
general plan should look like; in fact, I asked for your opinion here:

http://archives.postgresql.org/pgsql-hackers/2009-12/msg01824.php

...Robert