Re: [v9.2] Fix leaky-view problem, part 2

On Fri, Jul 08, 2011 at 10:09:54AM +0100, Kohei KaiGai wrote:
> 2011/7/8 Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>:
> > On 08.07.2011 11:03, Kohei KaiGai wrote:
> >>
> >> 2011/7/7 Noah Misch<noah(at)2ndquadrant(dot)com>:
> >>>
> >>> Making a distinction based simply on the call being an operator vs. a
> >>> function
> >>> is a dead end. ?I see these options:
> >>>
> >>> 1. The user defining a security view can be assumed to trust the operator
> >>> class
> >>> members of indexes defined on the tables he references. ?Keep track of
> >>> which
> >>> those are and treat only them as non-leakable. ?This covers many
> >>> interesting
> >>> cases, but it's probably tricky to implement and/or costly at runtime.
> >>>
> >> It requires DBA massive amount of detailed knowledge about functions
> >> underlying
> >> operators used in a view. I don't think it is a realistic assumption.
> >>
> >>> 2. Add a pg_proc flag indicating whether the function is known leak-free.
> >>> Simple, but tedious and perhaps error-prone.
> >>>
> >> +1
> >
> > IMHO the situation from DBA's point of view is exactly opposite. Option two
> > requires deep knowledge of this leaky views issue. The DBA needs to inspect
> > any function he wants to mark as leak-free closely, and understand that
> > innocent-looking things like casts can cause leaks. That is not feasible in
> > practice. Option 1, however, requires no such knowledge. Operators used in
> > indexes are already expected to not throw errors, or you would get errors
> > when inserting certain values to the table, for example.
> >
> I might misread his description at first.
> Hmm. If we introduce DBA the scenario and the condition to push down qualifiers,
> it may be possible to explain more simply.
>
> A challenge of this approach is to determine what qualifier shall be
> used to index
> accesses in the stage of distribute_qual_to_rels(); prior to the
> optimizer's selection
> of access methods.
> Do you have any good idea, or suggestion?

Note that it does not matter whether we're actually doing an index scan -- a seq
scan with a filter using only leakproof operators is equally acceptable. What I
had in mind was to enumerate all operators in operator classes of indexes below
each security view. Those become the leak-free operators for that security
view. If the operator for an OpExpr is considered leak-free by all sources of
its operands, then we may push it down. That's purely a high-level sketch: I
haven't considered implementation concerns in any detail. The resulting
behavior could be surprising: adding an index may change a plan without the new
plan actually using the index.

I lean toward favoring the pg_proc flag. Functions like "texteq" will be taken
as leakproof even if no involved table has an index on a text column. It works
for functions that will never take a place in an operator class, like
length(text). When a user reports a qualifier not getting pushed down, the
answer is much more satisfying: "Run 'CREATE OR REPLACE FUNCTION
... I_DONT_LEAK' as a superuser." Compare to "Define an operator class that
includes the function, if needed, and create an otherwise-useless index." The
main disadvantage I see is the loss of policy locality. Only a superuser (or
maybe database owner?) can create or modify declared-leakproof functions, and
that decision applies throughout the database. However, I think the other
advantages clearly outweigh that loss.

Incidentally, whichever policy we choose here can also loosen the constraints on
qualifier order (part 1 of your original submission).