The attached patch is a revised one with the following updates:
- rebased to the latest CVS HEAD
- eliminated comments about code which already removed, such as "we had
ACL_xxx checks here, but it is moved to ac_xxx_create()", and some of
notifications are moved to the README.
(comments about LookupCreationNamespace() and CheckRelationOwnership())
- removed ac_relation_permission() invocation from OpenIntoRel()
because the default PG model uses the perspective CREATE TABLE AS is
an atomic operation, due to the defaultACL thread.
(It is already talked with Stephen, and agreed.)
- fixed two bugs:
* ac_index_create() didn't bypass checks on bootstraping mode.
* ac_schema_alter() didn't checks ACL_CREATE on changing owner.
Thanks,
Stephen Frost wrote:
> KaiGai,
>
> * KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
>> Please review the new revision, Thanks,
>
> In general, I'm pretty happy with this revision. You still have a
> number of places where you have comments about code which does not exist
> any more. For example, the comments about the check being removed from
> LookupCreationNamespace. I would recommend pulling out those comments
> and instead having a comment at the top of the function that says
> "namespace creation permission checks are handled in the individual
> object ac_*_create() routines".
>
> I don't like having comments that are about code which was removed.
> Some of these could be moved to the README if they aren't there already
> and they really need to be kept.
>
> There are some other grammatical and spelling issues in the comments,
> but I don't believe any of this should hold this patch up from being
> ready for committer. At a minimum, I think this really needs to have a
> committer comment on it to ensure we're going in the right direction.
> I'd be happy to continue working with KaiGai to review his changes going
> forward, either with the next set of SE-PG patches or reworking this one
> if necessary.
>
> Thanks,
>
> Stephen
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>