From 42260ffcfac54f11abbdb57522a4feaa523d0f51 Mon Sep 17 00:00:00 2001
From: Andy Fan <yizhi.fzh@alibaba-inc.com>
Date: Sun, 6 Mar 2022 14:20:55 +0800
Subject: [PATCH v4 3/6] Reduce some planning cost for deriving qual for EC
 filter feature.

Mainly changes includes:
1. Check if the qual is simple enough by checking rinfo->right_relids and
info->right_relids, save the pull_varnos of rinfo->clause calls.
2. check contain_volatile_functions against RestrictInfo, so that
the result can be shared with following calls.
3. By employing the RestictInfo->btreeineqfamility which is calculating.
with same round of calculating  RestrictInfo->mergeopfamilies. In this
way we save the some calls for syscache.
4. Calculating the opfamility and amstrategy at
distribute_filter_quals_to_eclass and cache the results in EquivalenceFilter.
if no suitable opfamility and amstrategy are found, bypass the qual immediately
and at last using the cached value generate_base_implied_equalities_no_const.

After this change, there is an testcase changed unexpectedly in equivclass.out
(compared with David's expectation file.)

create user regress_user_ectest;
grant select on ec0 to regress_user_ectest;
grant select on ec1 to regress_user_ectest;

set session authorization regress_user_ectest;

-- with RLS active, the non-leakproof a.ff = 43 clause is not treated
-- as a suitable source for an EquivalenceClass; currently, this is true
-- even though the RLS clause has nothing to do directly with the EC
explain (costs off)
regression->   select * from ec0 a, ec1 b
regression->   where a.ff = b.ff and a.ff = 43::bigint::int8alias1;

The b.ff = 43 is disappeared from ec1 b. But since it even didn't shown
before the EC filter, so I'm not sure my changes here make something wrong,
maybe fix a issue by accidental?
---
 src/backend/nodes/outfuncs.c             |  2 +
 src/backend/optimizer/path/equivclass.c  | 59 +++++++++++++-----------
 src/backend/optimizer/plan/initsplan.c   | 50 ++++----------------
 src/include/nodes/pathnodes.h            |  2 +
 src/test/regress/expected/equivclass.out |  6 +--
 5 files changed, 48 insertions(+), 71 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 4c384511c39..d29b64eb918 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2513,6 +2513,8 @@ _outEquivalenceFilter(StringInfo str, const EquivalenceFilter *node)
 	WRITE_OID_FIELD(ef_opno);
 	WRITE_BOOL_FIELD(ef_const_is_left);
 	WRITE_UINT_FIELD(ef_source_rel);
+	WRITE_OID_FIELD(opfamily);
+	WRITE_INT_FIELD(amstrategy);
 }
 
 static void
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index f9ae2785d60..915888a8486 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1252,19 +1252,17 @@ generate_base_implied_equalities_const(PlannerInfo *root,
 }
 
 /*
- * finds the opfamily and strategy number for the specified 'opno' and 'method'
- * access method. Returns True if one is found and sets 'family' and
- * 'amstrategy', or returns False if none are found.
+ * finds the operator id for the specified 'opno' and 'method' and 'opfamilies'
+ * Returns True if one is found and sets 'opfamily_p' and 'amstrategy_p' or returns
+ * False if none are found.
  */
 static bool
-find_am_family_and_stategy(Oid opno, Oid method, Oid *family, int *amstrategy)
+find_am_family_and_stategy(Oid opno, Oid method, List *opfamilies,
+						   Oid *opfamily_p, int *amstrategy_p)
 {
-	List *opfamilies;
 	ListCell *l;
 	int strategy;
 
-	opfamilies = get_opfamilies(opno, method);
-
 	foreach(l, opfamilies)
 	{
 		Oid opfamily = lfirst_oid(l);
@@ -1273,8 +1271,8 @@ find_am_family_and_stategy(Oid opno, Oid method, Oid *family, int *amstrategy)
 
 		if (strategy)
 		{
-			*amstrategy = strategy;
-			*family = opfamily;
+			*opfamily_p = opfamily;
+			*amstrategy_p = strategy;
 			return true;
 		}
 	}
@@ -1363,17 +1361,11 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
 			EquivalenceFilter *ef = (EquivalenceFilter *) lfirst(lc2);
 			Expr *leftexpr;
 			Expr *rightexpr;
-			int strategy;
 			Oid opno;
-			Oid family;
 
 			if (ef->ef_source_rel == relid)
 				continue;
 
-			if (!find_am_family_and_stategy(ef->ef_opno, BTREE_AM_OID,
-				&family, &strategy))
-				continue;
-
 			if (ef->ef_const_is_left)
 			{
 				leftexpr = (Expr *) ef->ef_const;
@@ -1385,10 +1377,10 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
 				rightexpr = (Expr *) ef->ef_const;
 			}
 
-			opno = get_opfamily_member(family,
+			opno = get_opfamily_member(ef->opfamily,
 										exprType((Node *) leftexpr),
 										exprType((Node *) rightexpr),
-										strategy);
+										ef->amstrategy);
 
 			if (opno == InvalidOid)
 				continue;
@@ -1399,7 +1391,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
 										rightexpr,
 										bms_copy(ec->ec_relids),
 										bms_copy(cur_em->em_nullable_relids),
-									 	ec->ec_min_security,
+										ec->ec_min_security,
 										ec->ec_below_outer_join,
 										false);
 		}
@@ -2007,9 +1999,12 @@ distribute_filter_quals_to_eclass(PlannerInfo *root, List *quallist)
 	 */
 	foreach(l, quallist)
 	{
-		OpExpr	   *opexpr = (OpExpr *) lfirst(l);
-		Expr	   *leftexpr = (Expr *) linitial(opexpr->args);
-		Expr	   *rightexpr = (Expr *) lsecond(opexpr->args);
+		RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
+		OpExpr *opexpr = (OpExpr *)(rinfo->clause);
+
+		Oid opfamily;
+		int amstrategy;
+
 		Const	   *constexpr;
 		Expr	   *varexpr;
 		Relids		exprrels;
@@ -2021,25 +2016,31 @@ distribute_filter_quals_to_eclass(PlannerInfo *root, List *quallist)
 		 * Determine if the the OpExpr is in the form "expr op const" or
 		 * "const op expr".
 		 */
-		if (IsA(leftexpr, Const))
+		if (bms_is_empty(rinfo->left_relids))
 		{
-			constexpr = (Const *) leftexpr;
-			varexpr = rightexpr;
+			constexpr = (Const *) get_leftop(rinfo->clause);
+			varexpr = (Expr *) get_rightop(rinfo->clause);
 			const_isleft = true;
+			exprrels = rinfo->right_relids;
 		}
 		else
 		{
-			constexpr = (Const *) rightexpr;
-			varexpr = leftexpr;
+			constexpr = (Const *) get_rightop(rinfo->clause);
+			varexpr = (Expr *) get_leftop(rinfo->clause);
 			const_isleft = false;
+			exprrels = rinfo->left_relids;
 		}
 
-		exprrels = pull_varnos(root, (Node *) varexpr);
-
 		/* should be filtered out, but we need to determine relid anyway */
 		if (!bms_get_singleton_member(exprrels, &relid))
 			continue;
 
+		if (!find_am_family_and_stategy(opexpr->opno, BTREE_AM_OID,
+										rinfo->btreeineqopfamilies,
+										&opfamily,
+										&amstrategy))
+			continue;
+
 		/* search for a matching eclass member in all eclasses */
 		foreach(l2, root->eq_classes)
 		{
@@ -2075,6 +2076,8 @@ distribute_filter_quals_to_eclass(PlannerInfo *root, List *quallist)
 					efilter->ef_const_is_left = const_isleft;
 					efilter->ef_opno = opexpr->opno;
 					efilter->ef_source_rel = relid;
+					efilter->opfamily = opfamily;
+					efilter->amstrategy = amstrategy;
 
 					ec->ec_filters = lappend(ec->ec_filters, efilter);
 					break;		/* Onto the next eclass */
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 7e355c94362..5a32c3987a0 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -651,44 +651,6 @@ create_lateral_join_info(PlannerInfo *root)
 	}
 }
 
-/*
- * is_simple_filter_qual
- *             Analyzes an OpExpr to determine if it may be useful as an
- *             EquivalenceFilter. Returns true if the OpExpr may be of some use, or
- *             false if it should not be used.
- */
-static bool
-is_simple_filter_qual(PlannerInfo *root, OpExpr *expr)
-{
-	Expr *leftexpr;
-	Expr *rightexpr;
-
-	if (!IsA(expr, OpExpr))
-			return false;
-
-	if (list_length(expr->args) != 2)
-			return false;
-
-	leftexpr = (Expr *) linitial(expr->args);
-	rightexpr = (Expr *) lsecond(expr->args);
-
-	/* XXX should we restrict these to simple Var op Const expressions? */
-	if (IsA(leftexpr, Const))
-	{
-		if (bms_membership(pull_varnos(root, (Node *) rightexpr)) == BMS_SINGLETON &&
-			!contain_volatile_functions((Node *) rightexpr))
-			return true;
-	}
-	else if (IsA(rightexpr, Const))
-	{
-		if (bms_membership(pull_varnos(root, (Node *) leftexpr)) == BMS_SINGLETON &&
-			!contain_volatile_functions((Node *) leftexpr))
-			return true;
-	}
-
-	return false;
-}
-
 /*****************************************************************************
  *
  *	  JOIN TREE PROCESSING
@@ -1678,6 +1640,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	bool		maybe_outer_join;
 	Relids		nullable_relids;
 	RestrictInfo *restrictinfo;
+	int	relid;
 
 	/*
 	 * Retrieve all relids mentioned within the clause.
@@ -2027,8 +1990,15 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	distribute_restrictinfo_to_rels(root, restrictinfo);
 
 	/* Check if the qual looks useful to harvest as an EquivalenceFilter */
-	if (filter_qual_list != NULL && is_simple_filter_qual(root, (OpExpr *) clause))
-		*filter_qual_list = lappend(*filter_qual_list, clause);
+	if (filter_qual_list != NULL &&
+		is_opclause(restrictinfo->clause) &&
+		!contain_volatile_functions((Node *)restrictinfo) && // Cachable
+		restrictinfo->btreeineqopfamilies != NIL &&  /* ineq expression */
+		/* simple & common enough filter, one side references one relation and the other one is a constant */
+		((bms_is_empty(restrictinfo->left_relids) && bms_get_singleton_member(restrictinfo->right_relids, &relid)) ||
+		 (bms_is_empty(restrictinfo->right_relids) && bms_get_singleton_member(restrictinfo->left_relids, &relid)))
+		)
+		*filter_qual_list = lappend(*filter_qual_list, restrictinfo);
 }
 
 /*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index dcae69635ce..e7b04211839 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1037,6 +1037,8 @@ typedef struct EquivalenceFilter
 	Oid			ef_opno;		/* Operator Oid of filter operator */
 	bool		ef_const_is_left; /* Is the Const on the left of the OpExrp? */
 	Index		ef_source_rel;	/* relid of originating relation. */
+	Oid			opfamily;
+	int			amstrategy;
 } EquivalenceFilter;
 
 /*
diff --git a/src/test/regress/expected/equivclass.out b/src/test/regress/expected/equivclass.out
index 92fcec1158b..980bd3817d3 100644
--- a/src/test/regress/expected/equivclass.out
+++ b/src/test/regress/expected/equivclass.out
@@ -407,14 +407,14 @@ set session authorization regress_user_ectest;
 explain (costs off)
   select * from ec0 a, ec1 b
   where a.ff = b.ff and a.ff = 43::bigint::int8alias1;
-                              QUERY PLAN                              
-----------------------------------------------------------------------
+                 QUERY PLAN                  
+---------------------------------------------
  Nested Loop
    ->  Index Scan using ec0_pkey on ec0 a
          Index Cond: (ff = '43'::int8alias1)
    ->  Index Scan using ec1_pkey on ec1 b
          Index Cond: (ff = a.ff)
-         Filter: ((f1 < '5'::int8alias1) AND (ff = '43'::int8alias1))
+         Filter: (f1 < '5'::int8alias1)
 (6 rows)
 
 reset session authorization;
-- 
2.21.0

