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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers