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

Tom Lane wrote:
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)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.

I read your comments again, but I still cannot understand the reason
why the current approah is worse than the way of input/output handler
replaced before.

If your opinion is unchanged now, could you tell me the point?

At least, the security system columns need to accept new security
labels in text format which are not on pg_security.
If type input handler translate the given string into internal
security identifier, it allows to make unnecessary entries.

For example:
SELECT 'valid_but_new_security_label'::regseclabel;

In the current implementation, this translation is done just before
an actual insertion or updates. So, it does not insert unrefered
entries and does not pollute pg_security under read-only transaction.

You suggested that heap_getsysattr() is wrong place for display
formatting of values. Its reason is still unclear for me.
The relationship between security identifier and security label is
similar to a data type with external storage, like TEXT.
It is not un-toasted on output handler. It is already untoasted
and its Datum hold a pointer to varlena object generated on runtime,
then textout() translate it into CString.
I think the "security_label" and "security_acl" is similar ones.
Can you consider the security identifiers are like a pointer which
indicates internal object, so these should be resolved before the
values are delivered to output handlers?

In addition, there are a few minor issues if we switch to the
input/output handler approach:

- What should it send/receive on binary type handler?
The security identifiers are purely internal representation. If we can
export them to outside of the system, it is nonsense, because we have
no assurance same security identifier has same label in other hosts.
In the older SE-PostgreSQL, it did not implement binary handler to
prevent binary output.

- Output handler cannot know what relation is the given security
identifier fetched from.
When the given security identifier is not valid, we need to prepare
an alternative TEXT or ACLs. The Row-level ACLs generates a default
ACLs like '{=rwdx/kaigai}', because it allows anything on tuples
without any ACLs. But it requires an information of who is owner
of the relation. It is not delivered to type i/o handlers, so we
have to consider another representation.

Indeed, it is possible to implement this feature with type i/o
handler approach. In fact, I had implemented the older version
in this one.
However, there are a few demerits, but I cannot find any merits
yet. So, I still hesitate to replace the current implementation
and believe the current implementation is better.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>