Re: Updates of SE-PostgreSQL 8.4devel patches (r1197)

Simon Riggs wrote:
> On Tue, 2008-11-18 at 11:51 +0900, KaiGai Kohei wrote:
>> Simon Riggs wrote:
>>>> The other is an issue on implementation. Even if row-level security is
>>>> disabled, tuples within pg_class, pg_attribute, pg_proc and pg_largeobject
>>>> has to hold its security context, because it means the security context of
>>>> tables, columns, procedure and largeobjects.
>>>> However, the length of a tuple is computed at heap_form_tuple(), and its
>>>> arguments don't contains the object id of relation. Thus, we cannot make
>>>> a decision whether the newly created tuple should have a security field, or
>>>> not. The heap_form_tuple() is invoked from 60 of functions. It might be
>>>> possible to change all the argument list to deliver relation id. But it
>>>> seems to me excess updates for the purpose.
>>> WITH OIDS seems to work well without problem. Why is this different?
>> It is not a problem, but things which take a decision.
>>
>> The heap_form_tuple() makes a new tuple according to the given
>> TupleDesc which has a bool variable of "tdhasoid".
>> This structure can be initialized at various functions. For example,
>> 47 of functions invoke CreateTemplateTupleDesc() which requires an
>> argument of "bool hasoid".
>> If we follow the way of WITH OIDS, it will be necessary to modify
>> the declaration and 47 of invocations, at least.
>>
>> I don't think your suggestion is a bad idea, except for the number
>> of modification on the core implementation.
> 
> We manage to include a NULL bitmap without doing this.

The heap_form_tuple() has an argument of "bool *isnull".
It is a hint of NULL bitmap.

> The hasoids option seems to be unused on most of those call points. How
> many of those call points would need security context?

If we focus on the CreateTemplateTupleDesc(), 5 of call points give
possibile "hasoid" argument, and rest of them always give "false".
I guess it will be same in the security context cases.
However, we have to change all the call points when the declaration
is changed.

> Another way would be to include a security context in all newly created
> tuples, but remove it during heap_update, heap_insert etc if it is
> unused by the relation. That seems more straightforward.

It is not a reasonable option.

The length of HeapTupleData is determined during heap_form_tuple(),
and it is unchanged later. Thus, we have to interpose here, as object
identifier doing.

>> Some of distributions now provides SELinux option, but not a default.
>> I know Debian, Ubuntu, Gentoo and SuSE are doing.
> 
> SUSE?

The "u" might be a large-letter.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei