Re: [v9.2] Fix Leaky View Problem

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.

> 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.

The documentation in general is not nearly adequate, at least IMHO.

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?

More when I've had more time to look at this...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company