Re: [v9.2] Fix Leaky View Problem

2011/11/2 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Wed, Nov 2, 2011 at 7:34 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> [ new patch, with example query plans ]
>
> I like the look of those query plans.
>
> Redefining the RangeTblEntry's relid field to be valid for either a
> table or a subquery that originated from a view seems problematic to
> me, though.  For one thing, it's hard to say how much other code
> assumes that field to be valid only for a table.  For example, you
> didn't update _readRangeTblEntry(), and I wouldn't bet on that being
> the only place that needs fixing.  For another thing, instead of
> changing the meaning of the relid field, you could just leave that
> alone and instead add a "bool security_barrier field" that caches the
> answer; ApplyRetrieveRule() has the Relation object and could set that
> field appropriately, and then subquery_was_security_barrier() wouldn't
> need a syscache lookup.
>
> Now, the obvious objection is that the security-barrier attribute
> might change between the time the RTE is created and the time that it
> gets used.  But if that's a danger, then presumably the whole view
> could also change, in which case the Query object would be pointing to
> the wrong data anyway.  I'm not sure I fully understand the details
> here, but it seems like it ought to be safe to cache the
> security_barrier attribute any place it's safe to cache the Query
> itself.  It certainly doesn't seem right to think that we might end up
> using a new value of the security_barrier attribute with an old query,
> or the other way around.  So something seems funky here.
>
The reason why I redefined the relid of RangeTblEntry is to avoid
the problem when security_barrier attribute get changed by concurrent
transactions between rewriter and planenr stage.

Of course, I'm not 100% sure whether we have a routine that assumes
valid relid of RangeTblEntry is regular table, or not, although we could
run the regression test correctly.

As I examined before, updates of the issued pg_class shall invalidate
prepared statements that assumed a particular security_barrier
(maybe, PlanCacheRelCallback does this work?), so it is unavailable
to use old plans based on old view definition.
If we want to avoid syscache lookup on subquery_was_security_barrier(),
I think it is a feasible idea to hold the value of security_barrier within RTE.

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