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

Reply via email to