From b19ec90394d5d933028355725754b4bb7a8c9094 Mon Sep 17 00:00:00 2001 From: "Andrey V. Lepikhov" Date: Wed, 6 Jul 2022 15:47:30 +0300 Subject: [PATCH 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 | 57 +++++++++++++----------- src/backend/optimizer/plan/initsplan.c | 50 +++++---------------- src/include/nodes/pathnodes.h | 2 + src/test/regress/expected/equivclass.out | 6 +-- 5 files changed, 47 insertions(+), 70 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index b19462c758..f31f1de983 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2674,6 +2674,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 89c7f0dc39..38bbc325b0 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -1234,19 +1234,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); @@ -1255,8 +1253,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; } } @@ -1345,17 +1343,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; @@ -1367,10 +1359,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; @@ -1989,9 +1981,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; @@ -2003,25 +1998,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) { @@ -2057,6 +2058,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 f86276b667..d7a173b327 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 f80b47ae2c..942a52fcac 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1179,6 +1179,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 92fcec1158..980bd3817d 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.37.0