I took another look at this and came up with some alternate comment
rewording. I also added a couple of additional comments that should
hopefully clarify the code a bit.
Regards,
Dean
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
new file mode 100644
index 58d78e6..9e05fa8
*** 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,1990 ----
* 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 are passed Var nodes, and therefore 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;
/*
--- 2011,2017 ----
/* 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 84d58ae..0098375
*** 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 Vars passed to non-leakproof functions
*****************************************************************************/
/*
! * 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 the clause contains any non-leakproof functions that are
! * passed Var nodes of the current query level, and which might therefore leak
! * data. Qualifiers from outside a security_barrier view that might leak data
! * in this way should not be pushed down into the view in case the contents of
! * tuples intended to be filtered out by the view are revealed by the leaky
! * functions.
*/
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,1432 ----
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
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;
--- 1436,1460 ----
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
+ bool leaky;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
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;
}
}
--- 1463,1480 ----
{
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);
}
--- 1489,1495 ----
*/
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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers