Re: [v9.2] Fix leaky-view problem, part 2 - Mailing list pgsql-hackers

On Fri, Jul 08, 2011 at 10:09:54AM +0100, Kohei KaiGai wrote:
> 2011/7/8 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:
> > On 08.07.2011 11:03, Kohei KaiGai wrote:
> >>
> >> 2011/7/7 Noah Misch<noah@2ndquadrant.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).