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