Re: [PATCH] SE-PostgreSQL Updates rev.2096

2009/6/30 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)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.

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

To put some numbers around this:

$ cat *.patch | diffstat | tail -1
219 files changed, 11819 insertions(+), 29 deletions(-), 857 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.

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.

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.

Thanks,

...Robert