A while ago [1] I proposed an enhancement to the way qual pushdown
safety is decided in RLS / security barrier views. Currently we just
test for the presence of leaky functions in the qual, but it is
possible to do better than that, by further testing if the leaky
function is actually being passed information that we don't want to be
leaked.
Attached is a patch that does that, allowing restriction clause quals
to be pushed down into subquery RTEs if they contain leaky functions,
provided that the arglists of those leaky functions contain no Vars
(such Vars would necessarily refer to output columns of the subquery,
which is the data that must not be leaked).
An example of the sort of query this will help optimise is:
SELECT * FROM table_with_rls WHERE created > now() - '1 hour'::interval;
where currently this qual cannot be pushed down because neither now()
nor timestamptz_mi_interval() are leakproof, but since they are not
being passed any data from the table, they can't actually leak
anything, so the qual can be safely pushed down, allowing indexes to
be used if available.
In fact the majority of builtin functions aren't marked leakproof, and
probably most user functions aren't either, so this could potentially
be useful in a wide range of real-world queries, where it is common to
write quals of the form <column> <operator> <expression>, and the
expression may contain leaky functions.
Regards,
Dean
[1]
http://www.postgresql.org/message-id/caezatcwkcpfwilocnmfmszklgoabz7axkmgz-mk83gd3jg8...@mail.gmail.com
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
new file mode 100644
index 58d78e6..4433468
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
*************** targetIsInAllPartitionLists(TargetEntry
*** 1982,1988 ****
* 2. If unsafeVolatile is set, the qual must not contain any volatile
* functions.
*
! * 3. If unsafeLeaky is set, the qual must not contain any leaky functions.
*
* 4. The qual must not refer to the whole-row output of the subquery
* (since there is no easy way to name that within the subquery itself).
--- 1982,1989 ----
* 2. If unsafeVolatile is set, the qual must not contain any volatile
* functions.
*
! * 3. If unsafeLeaky is set, the qual must not contain any leaky functions
! * that might reveal values from the subquery as side effects.
*
* 4. The qual must not refer to the whole-row output of the subquery
* (since there is no easy way to name that within the subquery itself).
*************** qual_is_pushdown_safe(Query *subquery, I
*** 2009,2015 ****
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaky_functions(qual))
return false;
/*
--- 2010,2016 ----
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
! contain_leaked_vars(qual))
return false;
/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
new file mode 100644
index b340b01..7f82d54
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** static bool contain_mutable_functions_wa
*** 97,103 ****
static bool contain_volatile_functions_walker(Node *node, void *context);
static bool contain_volatile_functions_not_nextval_walker(Node *node, void *context);
static bool contain_nonstrict_functions_walker(Node *node, void *context);
! static bool contain_leaky_functions_walker(Node *node, void *context);
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
--- 97,103 ----
static bool contain_volatile_functions_walker(Node *node, void *context);
static bool contain_volatile_functions_not_nextval_walker(Node *node, void *context);
static bool contain_nonstrict_functions_walker(Node *node, void *context);
! static bool contain_leaked_vars_walker(Node *node, void *context);
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
*************** contain_nonstrict_functions_walker(Node
*** 1318,1343 ****
}
/*****************************************************************************
! * Check clauses for non-leakproof functions
*****************************************************************************/
/*
! * contain_leaky_functions
! * Recursively search for leaky functions within a clause.
*
! * Returns true if any function call with side-effect may be present in the
! * clause. Qualifiers from outside the a security_barrier view should not
! * be pushed down into the view, lest the contents of tuples intended to be
! * filtered out be revealed via side effects.
*/
bool
! contain_leaky_functions(Node *clause)
{
! return contain_leaky_functions_walker(clause, NULL);
}
static bool
! contain_leaky_functions_walker(Node *node, void *context)
{
if (node == NULL)
return false;
--- 1318,1347 ----
}
/*****************************************************************************
! * Check clauses for non-leakproof functions that might leak Vars
*****************************************************************************/
/*
! * contain_leaked_vars
! * Recursively scan a clause to discover whether it contains any Var
! * nodes (of the current query level) that are passed as arguments to
! * leaky functions.
*
! * Returns true if any function call with side effects may be present in the
! * clause and that function's arguments contain any Var nodes of the current
! * query level. Qualifiers from outside a security_barrier view that leak
! * Var nodes in this way should not be pushed down into the view, lest the
! * contents of tuples intended to be filtered out be revealed via the side
! * effects.
*/
bool
! contain_leaked_vars(Node *clause)
{
! return contain_leaked_vars_walker(clause, NULL);
}
static bool
! contain_leaked_vars_walker(Node *node, void *context)
{
if (node == NULL)
return false;
*************** contain_leaky_functions_walker(Node *nod
*** 1369,1375 ****
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid))
return true;
}
break;
--- 1373,1380 ----
{
FuncExpr *expr = (FuncExpr *) node;
! if (!get_func_leakproof(expr->funcid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1381,1387 ****
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1386,1393 ----
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1391,1397 ****
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid))
return true;
}
break;
--- 1397,1404 ----
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
! if (!get_func_leakproof(expr->opfuncid) &&
! contain_var_clause((Node *) expr->args))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1401,1415 ****
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1408,1428 ----
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1419,1432 ****
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! if (!get_func_leakproof(funcid))
! return true;
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! if (!get_func_leakproof(funcid))
return true;
}
break;
--- 1432,1452 ----
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
! leaky = !get_func_leakproof(funcid);
!
! if (!leaky)
! {
! getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
! leaky = !get_func_leakproof(funcid);
! }
!
! if (leaky &&
! contain_var_clause((Node *) expr->arg))
return true;
}
break;
*************** contain_leaky_functions_walker(Node *nod
*** 1435,1446 ****
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
! foreach(opid, rcexpr->opnos)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid))
return true;
}
}
--- 1455,1472 ----
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
+ ListCell *larg;
+ ListCell *rarg;
! forthree(opid, rcexpr->opnos,
! larg, rcexpr->largs,
! rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));
! if (!get_func_leakproof(funcid) &&
! (contain_var_clause((Node *) lfirst(larg)) ||
! contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}
*************** contain_leaky_functions_walker(Node *nod
*** 1455,1461 ****
*/
return true;
}
! return expression_tree_walker(node, contain_leaky_functions_walker,
context);
}
--- 1481,1487 ----
*/
return true;
}
! return expression_tree_walker(node, contain_leaked_vars_walker,
context);
}
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
new file mode 100644
index 160045c..3d04ac2
*** a/src/include/optimizer/clauses.h
--- b/src/include/optimizer/clauses.h
*************** extern bool contain_mutable_functions(No
*** 63,69 ****
extern bool contain_volatile_functions(Node *clause);
extern bool contain_volatile_functions_not_nextval(Node *clause);
extern bool contain_nonstrict_functions(Node *clause);
! extern bool contain_leaky_functions(Node *clause);
extern Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
--- 63,69 ----
extern bool contain_volatile_functions(Node *clause);
extern bool contain_volatile_functions_not_nextval(Node *clause);
extern bool contain_nonstrict_functions(Node *clause);
! extern bool contain_leaked_vars(Node *clause);
extern Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out
new file mode 100644
index 82d510d..7f57526
*** a/src/test/regress/expected/select_views.out
--- b/src/test/regress/expected/select_views.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(4 rows)
--
+ -- scenario: qualifiers can be pushed down if they contain leaky functions,
+ -- provided they aren't passed data from inside the view.
+ --
+ SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd123
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => beafsteak
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => hamburger
+ cid | name | tel | passwd
+ -----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+ (1 row)
+
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+ ---------------------------------------------------------------------------------------------
+ Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text))
+ (2 rows)
+
+ SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd123
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd
+ cid | name | tel | passwd
+ -----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+ (1 row)
+
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+ --------------------------------------------------------------------------------
+ Subquery Scan on v
+ Filter: f_leak(v.passwd)
+ -> Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text))
+ (4 rows)
+
+ --
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as
-- possible as we can.
diff --git a/src/test/regress/expected/select_views_1.out b/src/test/regress/expected/select_views_1.out
new file mode 100644
index ce22bfa..5275ef0
*** a/src/test/regress/expected/select_views_1.out
--- b/src/test/regress/expected/select_views_1.out
*************** EXPLAIN (COSTS OFF) SELECT * FROM my_pro
*** 1349,1354 ****
--- 1349,1400 ----
(4 rows)
--
+ -- scenario: qualifiers can be pushed down if they contain leaky functions,
+ -- provided they aren't passed data from inside the view.
+ --
+ SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd123
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => beafsteak
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => hamburger
+ cid | name | tel | passwd
+ -----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+ (1 row)
+
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+ ---------------------------------------------------------------------------------------------
+ Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text))
+ (2 rows)
+
+ SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd123
+ NOTICE: f_leak => passwd
+ NOTICE: f_leak => passwd
+ cid | name | tel | passwd
+ -----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+ (1 row)
+
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+ --------------------------------------------------------------------------------
+ Subquery Scan on v
+ Filter: f_leak(v.passwd)
+ -> Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text))
+ (4 rows)
+
+ --
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as
-- possible as we can.
diff --git a/src/test/regress/sql/select_views.sql b/src/test/regress/sql/select_views.sql
new file mode 100644
index da35623..3b74ab9
*** a/src/test/regress/sql/select_views.sql
--- b/src/test/regress/sql/select_views.sql
*************** SELECT * FROM my_property_secure WHERE f
*** 96,101 ****
--- 96,115 ----
EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd);
--
+ -- scenario: qualifiers can be pushed down if they contain leaky functions,
+ -- provided they aren't passed data from inside the view.
+ --
+ SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+
+ SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+
+ --
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as
-- possible as we can.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
