Re: [PATCH] ACE Framework - Database, Schema

2009/12/14 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> Robert Haas wrote:
>> 2009/12/13 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>>> Just to name a few really obvious problems (I only looked at the
>>>> 01-database patch):
>>>>
>>>> 1. We have been talking for several days about the need to make the
>>>> initial patch in this area strictly a code cleanup patch.  Is this
>>>> cleaner than the code that it is replacing?  Is it even making an
>>>> attempt to conform to that mandate?
>>> Even if it is unclear whether the current form is more clear than the
>>> current inlined pg_xxx_aclcheck() form, or not, it will obviously
>>> provide a set of common entry points for upcoming enhanced security
>>> providers.
>>> Eventually, it is more clear than enumeration of #ifdef ... #endif
>>> blocks for SELinux, Smack, Solaris-TX and others.
>>
>> Right, but it will also not get committed.  :-(
>
> The framework will be necessary to get them committed.
> Which is an egg, and which is a chicken? :-(

We've been around that path a few times, but that's not my point here.
Doing the framework first makes a lot of sense; the problem is that
we just had a design discussion regarding that framework and you've
chosen to do something other than what was discussed. Did you not
read that discussion? Did you not understand it?

Unfortunately, what this project has turned into is a very long series
of patch submissions that all basically have the same problems. The
code is messy. It doesn't conform to project style. It embeds
undiscussed design assumptions that the community does not endorse.
It has poorly factored interfaces that may serve SE-PostgreSQL
adequately for the moment but are likely to require constant
rejiggering as new problems arise. It is filled with shims that don't
accomplish anything useful and other places where useful shims are
left out.

Now, it may be that even if you respond to all of the comments and fix
all of the issues that people are concerned about, the patch still
won't get committed. As you know, Tom is very skeptical that the
user-base for this feature is large enough to warrant the work that it
will take. But my point is that you seem to be very deliberately not
fixing those problems, and I don't see how we're going to make any
progress that way. Many of the problems that I found in my review of
this patch were covered in design discussions that happened this week,
and yet the patch shows almost no evidence of those design
discussions.

Frankly, I find that kind of depressing.

...Robert