2011/9/24 Robert Haas <robertmhaas@gmail.com>:
> On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
>> I updated the patches of fix-leaky-view problem, according to the
>> previous discussion.
>> The "NOLEAKY" option was replaced by "LEAKPROOF" option, and several regression
>> test cases were added. Rest of stuffs are unchanged.
>
> You have a leftover reference to NOLEAKY.
>
Oops, I'll fix it.
>> For convenience of reviewer, below is summary of these patches:
>>
>> The Part-1 implements corresponding SQL syntax stuffs which are
>> "security_barrier"
>> reloption of views, and "LEAKPROOF" option on creation of functions to be stored
>> new pg_proc.proleakproof field.
>
> The way you have this implemented, we just blow away all view options
> whenever we do CREATE OR REPLACE VIEW. Is that the behavior we want?
> If a security_barrier view gets accidentally turned into a
> non-security_barrier view, doesn't that create a security_hole?
>
> I'm also wondering if the way you're using ResetViewOptions() is the
> right way to handle this anyhow. Isn't that going to update pg_class
> twice? I guess that's probably harmless from a performance
> standpoint, but wouldn't it be better not to? I guess we could define
> something like AT_ReplaceRelOptions to handle this case.
>
IIRC, we had a discussion that the behavior should follow the case
when a view is newly defined, even if it would be replaced actually.
However, it seems to me consistent way to keep existing setting
as long as user does not provide new option explicitly.
If so, I think AT_ReplaceRelOptions enables to simplify the code
to implement such a behavior.
> The documentation in general is not nearly adequate, at least IMHO.
>
Do you think the description is poor to introduce the behavior changes
corresponding to security_barrier option?
OK, I'll try to update the documentation.
> I'm a bit nervous about storing security_barrier in the RTE. What
> happens to stored rules if the security_barrier option gets change
> later?
>
The rte->security_barrier is evaluated when a query referencing security
views get expanded. So, rte->security_barrier is not stored to catalog.
Even if security_barrier option was changed after PREPARE statement
with references to security view, our existing mechanism re-evaluate
the query on EXECUTE, thus, it shall be executed as we expected.
(As an aside, I didn't know this mechanism and surprised at EXECUTE
works correctly, even if VIEW definition was changed after PREPARE.)
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
