Robert Haas wrote:
2009/6/30 KaiGai Kohei <kai...@ak.jp.nec.com>:
The SE-PostgreSQL patches are updated as follows:

01) http://sepgsql.googlecode.com/files/sepgsql-01-sysatt-8.4-r2096.patch
02) http://sepgsql.googlecode.com/files/sepgsql-02-core-8.4-r2096.patch
03) http://sepgsql.googlecode.com/files/sepgsql-03-gram-8.4-r2096.patch
04) http://sepgsql.googlecode.com/files/sepgsql-04-writable-8.4-r2096.patch
05) http://sepgsql.googlecode.com/files/sepgsql-05-rowlevel-8.4-r2096.patch
06) http://sepgsql.googlecode.com/files/sepgsql-06-perms-8.4-r2096.patch
07) http://sepgsql.googlecode.com/files/sepgsql-07-extra-8.4-r2096.patch
08) http://sepgsql.googlecode.com/files/sepgsql-08-utils-8.4-r2096.patch
09) http://sepgsql.googlecode.com/files/sepgsql-09-tests-8.4-r2096.patch
10) http://sepgsql.googlecode.com/files/sepgsql-10-docs-8.4-r2096.patch

KaiGai,

It appears that some things that you were previously asked to remove
for the first version, like row-level security, have crept back in
here.  I think that there is a clear consensus that what you have here
is too ambitious for a single commit.

Robert,

The row-level security is added at the fifth patch.
If you apply only the first three patches, it performs without row-level
security version.

I don't believe all the 10 patches get commited at the first commit fest.
If preferable, I can agree to forcus on the first three patches on the first
commit fest, according to the step-by-step approach, previously suggested.

 01 ... It provides a facility to manage security labels of database obejcts.
 02 ... It provides a core access control stuff including communication with 
kernel.
 03 ... It provides SECURITY_LABEL option in some of CREATE/ALTER statement.

http://archives.postgresql.org/message-id/20090311135413.gg4...@alvh.no-ip.org
http://archives.postgresql.org/message-id/336.1236792...@sss.pgh.pa.us

To put some numbers around this:

$ cat *.patch | diffstat | tail -1
 219 files changed, 11819 insertions(+), 29 deletions(-), 857 modifications(!)

Some of files are parched by multiple patches.

It is more correct estimation.
  $ diffstat sepgsql-00-full-8.4.0-r2096.patch.gz
  165 files changed, 11788 insertions(+), 2 deletions(-), 657 modifications(!)

For comparison:

Infrastructure Changes for Recovery
 8 files changed, 912 insertions(+), 183 deletions(-)
Window Functions
 92 files changed, 6631 insertions(+), 232 deletions(-)
WITH/WITH RECURSIVE
 77 files changed, 5826 insertions(+), 246 deletions(-)

Based on that, it looks to me like you should really aim to have a
patch set that changes AT MOST 5-6K lines, and less would be improve
your odds of having something accepted.  You can always submit
follow-on patches once the basic implementation is in, but I think I'm
reflecting the shared view of those who have looked at this in the
past when I say that it's never going to get committed in this form.

Scale of the first three patches is as follows:
$ diffstat sepgsql-01-sysatt-8.4.0-r2095.patch \
           sepgsql-02-core-8.4.0-r2095.patch \
           sepgsql-03-gram-8.4.0-r2095.patch
110 files changed, 5162 insertions(+), 2 deletions(-), 308 modifications(!)

The SE-PostgreSQL without row-level security version changes about 5-6K lines.
I can agree it may be a maximum scale in a single commit fest.

Just to be clear, the fact that you have split this up into multiple,
separate patch FILES is of no value at all, because they are
interdependent.  For example, I just took a quick look at the "01"
patch and it obviously includes code that is only necessary for
row-level security.  Nothing is going to get committed here unless it
is a self-contained change that does not presume that there will be
further commits in the future.

I considered the separated patches reflects the step-by-step approach
previously suggested. At least, the minimum SE-PostgreSQL can works
with the first two patches.
The purpose of 01 patch is to provides a facility to manage security
labels of any database objects, including tables, columns and so on.
However, indeed, I could find a part only necessary for row-level stuff,
such as definitions of "security_labels" and "security_acl".
I can agree to move them to the later patch.

Unless this is resubmitted in a form that complies with the changes
that were requested the last time it was reviewed, there is really no
point in reviewing it again.

Right now, I think it is preferable to focus on the first three patches
at the first commit fest.

What's your opinion?
--
KaiGai Kohei <kai...@kaigai.gr.jp>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to