Re: [PATCH] Reworks for Access Control facilities (r2311)

Stephen Frost wrote:
>>> I know it doesn't hide existence of major database objects. Depending
>>> on the situation, there might be other information that could be leaked.
>>> I realize that's not the case here, but I still want to catch and
>>> document any behavioral changes, even if it's clear they shouldn't be an
>>> issue.
>> I agree that it should be documented.
>> Where should I document them on? I guess the purpose of the description
>> is to inform these behavior changes for users, not only developers.
>> The official documentation sgml? wiki.postgresql.org? or, source code
>> comments are enough?
>
> What I would suggest is having a README or similar which accompanies the
> patch. This could then be included by reference in the commit message,
> or directly in the commit depending on what the committer prefers. Or,
> it could just go into the mailing list and commitfest archives. The
> point is to make sure the committer understands and isn't suprised when
> reviewing the changes and comes across places where the code changes
> result in a behaviour change.
>
> If the changes are significant enough (and I don't think they will be,
> to be honest..), they should be included by the committer in the commit
> message and then picked up by Bruce, et al, when the release notes are
> developed. I don't believe it needs to be in the formal PG
> documentation, unless there's something documented there today which is
> changing (very unlikely..).

It may be good idea to put src/backend/security/README.

I'm not clear whether the commit message or release note is appropriate.
(it is unnoticeable if commit message, it is too details for release note.)

> No, no. What I was suggesting and what I think we already do in most
> places (but not everywhere and it's not really a policy) is this:
>
> CreateXXXX()
> {
> namespaceId = LookupCreationNamespace();
> tablespaceId = get_tablespace_oid();
> ac_xxx_create();
> :
> values[ ... ] = ObjectIdGetDatum(namespaceId);
> values[ ... ] = ObjectIdGetDatum(tablespaceId);
> simple_heap_insert();
> :
> }
>
> Which I think is what you're doing with this, it just might be a change
> from what was done before when there were multiple permission checks
> done.

Ahh, it was a communication bug. we talked same thing.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>