From 9fdcb7aba6078788bcd23670f955c0dd8e60f493 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Wed, 10 Mar 2021 22:57:33 +1300 Subject: [PATCH v16 1/5] Cache PathTarget and RestrictInfo's volatility This aims to can reduce the number of times we make calls to contain_volatile_functions(). This really does not save us much with the existing set of calls to contain_volatile_functions(), however, it will save a significant number of calls in an upcoming patch which must check this during the join search. --- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 2 + src/backend/optimizer/path/allpaths.c | 40 ++++++++++--------- src/backend/optimizer/plan/initsplan.c | 2 +- src/backend/optimizer/plan/planner.c | 2 +- src/backend/optimizer/util/clauses.c | 47 +++++++++++++++++++++++ src/backend/optimizer/util/restrictinfo.c | 2 + src/backend/optimizer/util/tlist.c | 10 +++++ src/include/nodes/pathnodes.h | 16 +++++++- 9 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index da91cbd2b1..493a856745 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2310,6 +2310,7 @@ _copyRestrictInfo(const RestrictInfo *from) COPY_SCALAR_FIELD(can_join); COPY_SCALAR_FIELD(pseudoconstant); COPY_SCALAR_FIELD(leakproof); + COPY_SCALAR_FIELD(has_volatile); COPY_SCALAR_FIELD(security_level); COPY_BITMAPSET_FIELD(clause_relids); COPY_BITMAPSET_FIELD(required_relids); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 6493a03ff8..afd281ab5a 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2473,6 +2473,7 @@ _outPathTarget(StringInfo str, const PathTarget *node) WRITE_FLOAT_FIELD(cost.startup, "%.2f"); WRITE_FLOAT_FIELD(cost.per_tuple, "%.2f"); WRITE_INT_FIELD(width); + WRITE_ENUM_FIELD(has_volatile_expr, VolatileFunctions); } static void @@ -2497,6 +2498,7 @@ _outRestrictInfo(StringInfo str, const RestrictInfo *node) WRITE_BOOL_FIELD(can_join); WRITE_BOOL_FIELD(pseudoconstant); WRITE_BOOL_FIELD(leakproof); + WRITE_ENUM_FIELD(has_volatile, VolatileFunctions); WRITE_UINT_FIELD(security_level); WRITE_BITMAPSET_FIELD(clause_relids); WRITE_BITMAPSET_FIELD(required_relids); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index d73ac562eb..e2510235ef 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -134,7 +134,8 @@ static void check_output_expressions(Query *subquery, static void compare_tlist_datatypes(List *tlist, List *colTypes, pushdown_safety_info *safetyInfo); static bool targetIsInAllPartitionLists(TargetEntry *tle, Query *query); -static bool qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, +static bool qual_is_pushdown_safe(Query *subquery, Index rti, + RestrictInfo *rinfo, pushdown_safety_info *safetyInfo); static void subquery_push_qual(Query *subquery, RangeTblEntry *rte, Index rti, Node *qual); @@ -2177,11 +2178,12 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel, foreach(l, rel->baserestrictinfo) { RestrictInfo *rinfo = (RestrictInfo *) lfirst(l); - Node *clause = (Node *) rinfo->clause; if (!rinfo->pseudoconstant && - qual_is_pushdown_safe(subquery, rti, clause, &safetyInfo)) + qual_is_pushdown_safe(subquery, rti, rinfo, &safetyInfo)) { + Node *clause = (Node *)rinfo->clause; + /* Push it down */ subquery_push_qual(subquery, rte, rti, clause); } @@ -3390,37 +3392,39 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query) } /* - * qual_is_pushdown_safe - is a particular qual safe to push down? + * qual_is_pushdown_safe - is a particular rinfo safe to push down? * - * qual is a restriction clause applying to the given subquery (whose RTE + * rinfo is a restriction clause applying to the given subquery (whose RTE * has index rti in the parent query). * * Conditions checked here: * - * 1. The qual must not contain any SubPlans (mainly because I'm not sure - * it will work correctly: SubLinks will already have been transformed into - * SubPlans in the qual, but not in the subquery). Note that SubLinks that - * transform to initplans are safe, and will be accepted here because what - * we'll see in the qual is just a Param referencing the initplan output. + * 1. rinfo's clause must not contain any SubPlans (mainly because it's + * unclear that it will work correctly: SubLinks will already have been + * transformed into SubPlans in the qual, but not in the subquery). Note that + * SubLinks that transform to initplans are safe, and will be accepted here + * because what we'll see in the qual is just a Param referencing the initplan + * output. * - * 2. If unsafeVolatile is set, the qual must not contain any volatile + * 2. If unsafeVolatile is set, rinfo's clause 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. + * 3. If unsafeLeaky is set, rinfo's clause 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 + * 4. rinfo's clause must not refer to the whole-row output of the subquery * (since there is no easy way to name that within the subquery itself). * - * 5. The qual must not refer to any subquery output columns that were + * 5. rinfo's clause must not refer to any subquery output columns that were * found to be unsafe to reference by subquery_is_pushdown_safe(). */ static bool -qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, +qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo, pushdown_safety_info *safetyInfo) { bool safe = true; + Node *qual = (Node *) rinfo->clause; List *vars; ListCell *vl; @@ -3430,7 +3434,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, /* Refuse volatile quals if we found they'd be unsafe (point 2) */ if (safetyInfo->unsafeVolatile && - contain_volatile_functions(qual)) + contain_volatile_functions((Node *) rinfo)) return false; /* Refuse leaky quals if told to (point 3) */ diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 02f813cebd..efca702891 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -2653,7 +2653,7 @@ check_mergejoinable(RestrictInfo *restrictinfo) if (list_length(((OpExpr *) clause)->args) != 2) return; - opno = ((OpExpr *) clause)->opno; + opno = ((OpExpr *)clause)->opno; leftarg = linitial(((OpExpr *) clause)->args); if (op_mergejoinable(opno, exprType(leftarg)) && diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 424d25cbd5..40476cc18d 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -5903,7 +5903,7 @@ make_sort_input_target(PlannerInfo *root, col_is_srf[i] = true; have_srf = true; } - else if (contain_volatile_functions((Node *) expr)) + else if (contain_volatile_functions((Node *)expr)) { /* Unconditionally postpone */ postpone_col[i] = true; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 7e25f94293..bda6e58b5d 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -487,6 +487,53 @@ contain_volatile_functions_walker(Node *node, void *context) return true; } + if (IsA(node, RestrictInfo)) + { + RestrictInfo *rinfo = (RestrictInfo *) node; + + if (rinfo->has_volatile == VOLATILITY_NOVOLATILE) + return false; + else if (rinfo->has_volatile == VOLATILITY_VOLATILE) + return true; + else + { + bool hasvolatile; + + hasvolatile = contain_volatile_functions_walker((Node *) rinfo->clause, + context); + if (hasvolatile) + rinfo->has_volatile = VOLATILITY_VOLATILE; + else + rinfo->has_volatile = VOLATILITY_NOVOLATILE; + + return hasvolatile; + } + } + + if (IsA(node, PathTarget)) + { + PathTarget *target = (PathTarget *) node; + + if (target->has_volatile_expr == VOLATILITY_NOVOLATILE) + return false; + else if (target->has_volatile_expr == VOLATILITY_VOLATILE) + return true; + else + { + bool hasvolatile; + + hasvolatile = contain_volatile_functions_walker((Node *) target->exprs, + context); + + if (hasvolatile) + target->has_volatile_expr = VOLATILITY_VOLATILE; + else + target->has_volatile_expr = VOLATILITY_NOVOLATILE; + + return hasvolatile; + } + } + /* * See notes in contain_mutable_functions_walker about why we treat * MinMaxExpr, XmlExpr, and CoerceToDomain as immutable, while diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index eb113d94c1..e247b41c20 100644 --- a/src/backend/optimizer/util/restrictinfo.c +++ b/src/backend/optimizer/util/restrictinfo.c @@ -137,6 +137,8 @@ make_restrictinfo_internal(PlannerInfo *root, else restrictinfo->leakproof = false; /* really, "don't know" */ + restrictinfo->has_volatile = VOLATILITY_UNKNOWN; + /* * If it's a binary opclause, set up left/right relids info. In any case * set up the total clause relids info. diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 89853a0630..7779aab44b 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -623,6 +623,9 @@ make_pathtarget_from_tlist(List *tlist) i++; } + /* cache whether the tlist has any volatile functions */ + target->has_volatile_expr = VOLATILITY_UNKNOWN; + return target; } @@ -724,6 +727,13 @@ add_column_to_pathtarget(PathTarget *target, Expr *expr, Index sortgroupref) target->sortgrouprefs = (Index *) palloc0(nexprs * sizeof(Index)); target->sortgrouprefs[nexprs - 1] = sortgroupref; } + + /* + * Set has_volatile_expr to UNKNOWN incase the new expr contains a + * volatile function. + */ + if (target->has_volatile_expr == VOLATILITY_NOVOLATILE) + target->has_volatile_expr = VOLATILITY_UNKNOWN; } /* diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 86405a274e..84e2fe186d 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1056,6 +1056,16 @@ typedef struct PathKey bool pk_nulls_first; /* do NULLs come before normal values? */ } PathKey; +/* + * VolatileFunctions -- allows nodes to cache their contain_volatile_functions + * properties. VOLATILITY_UNKNOWN means not yet determined. + */ +typedef enum VolatileFunctions +{ + VOLATILITY_UNKNOWN = 0, + VOLATILITY_VOLATILE, + VOLATILITY_NOVOLATILE +} VolatileFunctions; /* * PathTarget @@ -1087,6 +1097,8 @@ typedef struct PathTarget Index *sortgrouprefs; /* corresponding sort/group refnos, or 0 */ QualCost cost; /* cost of evaluating the expressions */ int width; /* estimated avg width of result tuples */ + VolatileFunctions has_volatile_expr; /* indicates if exprs contain any + * volatile functions. */ } PathTarget; /* Convenience macro to get a sort/group refno from a PathTarget */ @@ -1860,7 +1872,6 @@ typedef struct LimitPath LimitOption limitOption; /* FETCH FIRST with ties or exact number */ } LimitPath; - /* * Restriction clause info. * @@ -2017,6 +2028,9 @@ typedef struct RestrictInfo bool leakproof; /* true if known to contain no leaked Vars */ + VolatileFunctions has_volatile; /* to indicate if clause contains any + * volatile functions. */ + Index security_level; /* see comment above */ /* The set of relids (varnos) actually referenced in the clause: */ -- 2.27.0