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

From: KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers(at)postgresql(dot)org, simon(at)2ndQuadrant(dot)com
Subject: Re: Updates of SE-PostgreSQL 8.4devel patches (r1324)
Date: 2008-12-19 15:23:06
Message-ID: 494BBC5A.8030404@kaigai.gr.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2008-12-19 15:34:56 Re: Hot standby and b-tree killed items
Previous Message Grzegorz Jaskiewicz 2008-12-19 14:51:25 Re: possible bug in 8.4