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

Tom Lane wrote:
> KaiGai Kohei <kaigai@ak.jp.nec.com> writes:
>> Tom Lane wrote:
>>> This seems like a pretty bad idea that will eventually bite you in an
>>> uncomfortable place.  Lying about what datatype a field is is just not
>>> safe.
> 
>> Is it also correct for system attributes?
>> I don't think the format on storage has to be same as user visible one,
>> because it always fetched via heap_getsysattr().
> 
> That's *exactly* the kind of thinking that will get you in trouble.
> Where is it set in stone that system attributes are always fetched
> via heap_getsysattr?  In any case, this amounts to putting display
> formatting of the value into heap_getsysattr, which surely seems
> like the wrong place for it.

Is there any foundamental difference compared to the case when we
refers a field with external storage, like a text field?
We need it to be detoasted before it is delivered to output handler.
It is unclear for me why it is legal but my patch is illegal.

>> The prior version of SE-PostgreSQL has a similar idea.
>> It is a specific type to translate security context between text
>> and oid. But, it was opposed at CommitFest:May because its input
>> handler has to insert a new tuple when the given security context
>> is not found on pg_security.
> 
> Yeah, I do remember that discussion ... but an input handler with
> side-effects seems better than what this solution is sounding like.

I didn't replace the previous implementation blindly, I have a few
reasons that the current one is better than previous one.

For example, if an input handler has side-effects, what is happen in
the following query?
 SELECT 'valid_but_new_security_label'::regseclabel;

It looks like a read-only query, but the input handler can insert a new
tuple into pg_security. In addition, the newly inserted tuple may not
be refered any more. It is a waste of spaces.

I want to keep the number of entries in pg_security necessity minimum.
When is it actually needed to insert a new security label into pg_security?
It is a case when a inserted/updated tuple requires a new security label
which is not on pg_security at the moment. I don't think the type-casting
is a case for "necessity minimum".

In another example, how does it prevent users to apply the type to
store his data?
 CREATE TABLE t (     a    int,     b    text,     c    regseclabel );

Anyway, it seems to me the current input handler does not assume it
has a side-effect. So, I believe your suggestion at CommitFest:May
is correct, and the security label in text should be translated into
internal oid representation just before the given tuple actually
need to assign security identifier.

At least, I think using input handler is a regression.
Is there any other idea, if the current design is bad.

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>