summaryrefslogtreecommitdiff
path: root/src/backend/optimizer/path
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2021-01-21 15:37:23 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2021-01-21 15:37:23 -0500
commit55dc86eca70b1dc18a79c141b3567efed910329d (patch)
tree24edd3fe9c8d4017595782b980b6bc2191643e91 /src/backend/optimizer/path
parent920f853dc948b98a5dc96580c4ee011a302e33e4 (diff)
Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar.
Previously, pull_varnos() took the relids of a PlaceHolderVar as being equal to the relids in its contents, but that fails to account for the possibility that we have to postpone evaluation of the PHV due to outer joins. This could result in a malformed plan. The known cases end up triggering the "failed to assign all NestLoopParams to plan nodes" sanity check in createplan.c, but other symptoms may be possible. The right value to use is the join level we actually intend to evaluate the PHV at. We can get that from the ph_eval_at field of the associated PlaceHolderInfo. However, there are some places that call pull_varnos() before the PlaceHolderInfos have been created; in that case, fall back to the conservative assumption that the PHV will be evaluated at its syntactic level. (In principle this might result in missing some legal optimization, but I'm not aware of any cases where it's an issue in practice.) Things are also a bit ticklish for calls occurring during deconstruct_jointree(), but AFAICS the ph_eval_at fields should have reached their final values by the time we need them. The main problem in making this work is that pull_varnos() has no way to get at the PlaceHolderInfos. We can fix that easily, if a bit tediously, in HEAD by passing it the planner "root" pointer. In the back branches that'd cause an unacceptable API/ABI break for extensions, so leave the existing entry points alone and add new ones with the additional parameter. (If an old entry point is called and encounters a PHV, it'll fall back to using the syntactic level, again possibly missing some valid optimization.) Back-patch to v12. The computation is surely also wrong before that, but it appears that we cannot reach a bad plan thanks to join order restrictions imposed on the subquery that the PlaceHolderVar came from. The error only became reachable when commit 4be058fe9 allowed trivial subqueries to be collapsed out completely, eliminating their join order restrictions. Per report from Stephan Springl. Discussion: https://postgr.es/m/171041.1610849523@sss.pgh.pa.us
Diffstat (limited to 'src/backend/optimizer/path')
-rw-r--r--src/backend/optimizer/path/clausesel.c12
-rw-r--r--src/backend/optimizer/path/costsize.c2
-rw-r--r--src/backend/optimizer/path/equivclass.c17
-rw-r--r--src/backend/optimizer/path/indxpath.c61
-rw-r--r--src/backend/optimizer/path/pathkeys.c2
-rw-r--r--src/backend/optimizer/path/tidpath.c18
6 files changed, 65 insertions, 47 deletions
diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index 4f5b870d1bc..d263ecf0827 100644
--- a/src/backend/optimizer/path/clausesel.c
+++ b/src/backend/optimizer/path/clausesel.c
@@ -227,7 +227,7 @@ clauselist_selectivity_ext(PlannerInfo *root,
}
else
{
- ok = (NumRelids(clause) == 1) &&
+ ok = (NumRelids(root, clause) == 1) &&
(is_pseudo_constant_clause(lsecond(expr->args)) ||
(varonleft = false,
is_pseudo_constant_clause(linitial(expr->args))));
@@ -609,7 +609,7 @@ bms_is_subset_singleton(const Bitmapset *s, int x)
* restriction or join estimator. Subroutine for clause_selectivity().
*/
static inline bool
-treat_as_join_clause(Node *clause, RestrictInfo *rinfo,
+treat_as_join_clause(PlannerInfo *root, Node *clause, RestrictInfo *rinfo,
int varRelid, SpecialJoinInfo *sjinfo)
{
if (varRelid != 0)
@@ -643,7 +643,7 @@ treat_as_join_clause(Node *clause, RestrictInfo *rinfo,
if (rinfo)
return (bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
else
- return (NumRelids(clause) > 1);
+ return (NumRelids(root, clause) > 1);
}
}
@@ -860,7 +860,7 @@ clause_selectivity_ext(PlannerInfo *root,
OpExpr *opclause = (OpExpr *) clause;
Oid opno = opclause->opno;
- if (treat_as_join_clause(clause, rinfo, varRelid, sjinfo))
+ if (treat_as_join_clause(root, clause, rinfo, varRelid, sjinfo))
{
/* Estimate selectivity for a join clause. */
s1 = join_selectivity(root, opno,
@@ -896,7 +896,7 @@ clause_selectivity_ext(PlannerInfo *root,
funcclause->funcid,
funcclause->args,
funcclause->inputcollid,
- treat_as_join_clause(clause, rinfo,
+ treat_as_join_clause(root, clause, rinfo,
varRelid, sjinfo),
varRelid,
jointype,
@@ -907,7 +907,7 @@ clause_selectivity_ext(PlannerInfo *root,
/* Use node specific selectivity calculation function */
s1 = scalararraysel(root,
(ScalarArrayOpExpr *) clause,
- treat_as_join_clause(clause, rinfo,
+ treat_as_join_clause(root, clause, rinfo,
varRelid, sjinfo),
varRelid,
jointype,
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 380336518fe..aab06c7d213 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -1858,7 +1858,7 @@ cost_incremental_sort(Path *path,
* Check if the expression contains Var with "varno 0" so that we
* don't call estimate_num_groups in that case.
*/
- if (bms_is_member(0, pull_varnos((Node *) member->em_expr)))
+ if (bms_is_member(0, pull_varnos(root, (Node *) member->em_expr)))
{
unknown_varno = true;
break;
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index ab6eaaead12..0188c1e9a18 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -196,7 +196,8 @@ process_equivalence(PlannerInfo *root,
ntest->location = -1;
*p_restrictinfo =
- make_restrictinfo((Expr *) ntest,
+ make_restrictinfo(root,
+ (Expr *) ntest,
restrictinfo->is_pushed_down,
restrictinfo->outerjoin_delayed,
restrictinfo->pseudoconstant,
@@ -716,7 +717,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
/*
* Get the precise set of nullable relids appearing in the expression.
*/
- expr_relids = pull_varnos((Node *) expr);
+ expr_relids = pull_varnos(root, (Node *) expr);
nullable_relids = bms_intersect(nullable_relids, expr_relids);
newem = add_eq_member(newec, copyObject(expr), expr_relids,
@@ -1696,7 +1697,8 @@ create_join_clause(PlannerInfo *root,
*/
oldcontext = MemoryContextSwitchTo(root->planner_cxt);
- rinfo = build_implied_join_equality(opno,
+ rinfo = build_implied_join_equality(root,
+ opno,
ec->ec_collation,
leftem->em_expr,
rightem->em_expr,
@@ -1996,7 +1998,8 @@ reconsider_outer_join_clause(PlannerInfo *root, RestrictInfo *rinfo,
cur_em->em_datatype);
if (!OidIsValid(eq_op))
continue; /* can't generate equality */
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
innervar,
cur_em->em_expr,
@@ -2141,7 +2144,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
cur_em->em_datatype);
if (OidIsValid(eq_op))
{
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
leftvar,
cur_em->em_expr,
@@ -2156,7 +2160,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
cur_em->em_datatype);
if (OidIsValid(eq_op))
{
- newrinfo = build_implied_join_equality(eq_op,
+ newrinfo = build_implied_join_equality(root,
+ eq_op,
cur_ec->ec_collation,
rightvar,
cur_em->em_expr,
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 9c069a213f9..ff536e6b24b 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -153,7 +153,8 @@ static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_boolean_index_clause(RestrictInfo *rinfo,
+static IndexClause *match_boolean_index_clause(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol, IndexOptInfo *index);
static IndexClause *match_opclause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
@@ -169,13 +170,16 @@ static IndexClause *get_index_clause_from_support(PlannerInfo *root,
int indexarg,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_saopclause_to_indexcol(RestrictInfo *rinfo,
+static IndexClause *match_saopclause_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *match_rowcompare_to_indexcol(RestrictInfo *rinfo,
+static IndexClause *match_rowcompare_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
-static IndexClause *expand_indexqual_rowcompare(RestrictInfo *rinfo,
+static IndexClause *expand_indexqual_rowcompare(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index,
Oid expr_op,
@@ -2305,7 +2309,7 @@ match_clause_to_indexcol(PlannerInfo *root,
opfamily = index->opfamily[indexcol];
if (IsBooleanOpfamily(opfamily))
{
- iclause = match_boolean_index_clause(rinfo, indexcol, index);
+ iclause = match_boolean_index_clause(root, rinfo, indexcol, index);
if (iclause)
return iclause;
}
@@ -2325,11 +2329,11 @@ match_clause_to_indexcol(PlannerInfo *root,
}
else if (IsA(clause, ScalarArrayOpExpr))
{
- return match_saopclause_to_indexcol(rinfo, indexcol, index);
+ return match_saopclause_to_indexcol(root, rinfo, indexcol, index);
}
else if (IsA(clause, RowCompareExpr))
{
- return match_rowcompare_to_indexcol(rinfo, indexcol, index);
+ return match_rowcompare_to_indexcol(root, rinfo, indexcol, index);
}
else if (index->amsearchnulls && IsA(clause, NullTest))
{
@@ -2368,7 +2372,8 @@ match_clause_to_indexcol(PlannerInfo *root,
* index's key, and if so, build a suitable IndexClause.
*/
static IndexClause *
-match_boolean_index_clause(RestrictInfo *rinfo,
+match_boolean_index_clause(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2438,7 +2443,7 @@ match_boolean_index_clause(RestrictInfo *rinfo,
IndexClause *iclause = makeNode(IndexClause);
iclause->rinfo = rinfo;
- iclause->indexquals = list_make1(make_simple_restrictinfo(op));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root, op));
iclause->lossy = false;
iclause->indexcol = indexcol;
iclause->indexcols = NIL;
@@ -2663,7 +2668,8 @@ get_index_clause_from_support(PlannerInfo *root,
{
Expr *clause = (Expr *) lfirst(lc);
- indexquals = lappend(indexquals, make_simple_restrictinfo(clause));
+ indexquals = lappend(indexquals,
+ make_simple_restrictinfo(root, clause));
}
iclause->rinfo = rinfo;
@@ -2684,7 +2690,8 @@ get_index_clause_from_support(PlannerInfo *root,
* which see for comments.
*/
static IndexClause *
-match_saopclause_to_indexcol(RestrictInfo *rinfo,
+match_saopclause_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2703,7 +2710,7 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
return NULL;
leftop = (Node *) linitial(saop->args);
rightop = (Node *) lsecond(saop->args);
- right_relids = pull_varnos(rightop);
+ right_relids = pull_varnos(root, rightop);
expr_op = saop->opno;
expr_coll = saop->inputcollid;
@@ -2751,7 +2758,8 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
* is handled by expand_indexqual_rowcompare().
*/
static IndexClause *
-match_rowcompare_to_indexcol(RestrictInfo *rinfo,
+match_rowcompare_to_indexcol(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index)
{
@@ -2796,14 +2804,14 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
* These syntactic tests are the same as in match_opclause_to_indexcol()
*/
if (match_index_to_operand(leftop, indexcol, index) &&
- !bms_is_member(index_relid, pull_varnos(rightop)) &&
+ !bms_is_member(index_relid, pull_varnos(root, rightop)) &&
!contain_volatile_functions(rightop))
{
/* OK, indexkey is on left */
var_on_left = true;
}
else if (match_index_to_operand(rightop, indexcol, index) &&
- !bms_is_member(index_relid, pull_varnos(leftop)) &&
+ !bms_is_member(index_relid, pull_varnos(root, leftop)) &&
!contain_volatile_functions(leftop))
{
/* indexkey is on right, so commute the operator */
@@ -2822,7 +2830,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
case BTLessEqualStrategyNumber:
case BTGreaterEqualStrategyNumber:
case BTGreaterStrategyNumber:
- return expand_indexqual_rowcompare(rinfo,
+ return expand_indexqual_rowcompare(root,
+ rinfo,
indexcol,
index,
expr_op,
@@ -2856,7 +2865,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
* but we split it out for comprehensibility.
*/
static IndexClause *
-expand_indexqual_rowcompare(RestrictInfo *rinfo,
+expand_indexqual_rowcompare(PlannerInfo *root,
+ RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index,
Oid expr_op,
@@ -2926,7 +2936,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
if (expr_op == InvalidOid)
break; /* operator is not usable */
}
- if (bms_is_member(index->rel->relid, pull_varnos(constop)))
+ if (bms_is_member(index->rel->relid, pull_varnos(root, constop)))
break; /* no good, Var on wrong side */
if (contain_volatile_functions(constop))
break; /* no good, volatile comparison value */
@@ -3036,7 +3046,8 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
matching_cols);
rc->rargs = list_truncate(copyObject(non_var_args),
matching_cols);
- iclause->indexquals = list_make1(make_simple_restrictinfo((Expr *) rc));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root,
+ (Expr *) rc));
}
else
{
@@ -3050,7 +3061,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
copyObject(linitial(non_var_args)),
InvalidOid,
linitial_oid(clause->inputcollids));
- iclause->indexquals = list_make1(make_simple_restrictinfo(op));
+ iclause->indexquals = list_make1(make_simple_restrictinfo(root, op));
}
}
@@ -3667,7 +3678,9 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
* specified index column matches a boolean restriction clause.
*/
bool
-indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
+indexcol_is_bool_constant_for_query(PlannerInfo *root,
+ IndexOptInfo *index,
+ int indexcol)
{
ListCell *lc;
@@ -3689,7 +3702,7 @@ indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
continue;
/* See if we can match the clause's expression to the index column */
- if (match_boolean_index_clause(rinfo, indexcol, index))
+ if (match_boolean_index_clause(root, rinfo, indexcol, index))
return true;
}
@@ -3801,10 +3814,10 @@ match_index_to_operand(Node *operand,
* index: the index of interest
*/
bool
-is_pseudo_constant_for_index(Node *expr, IndexOptInfo *index)
+is_pseudo_constant_for_index(PlannerInfo *root, Node *expr, IndexOptInfo *index)
{
/* pull_varnos is cheaper than volatility check, so do that first */
- if (bms_is_member(index->rel->relid, pull_varnos(expr)))
+ if (bms_is_member(index->rel->relid, pull_varnos(root, expr)))
return false; /* no good, contains Var of table */
if (contain_volatile_functions(expr))
return false; /* no good, volatile comparison value */
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 3ebc57a1545..bd9a176d7d3 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -598,7 +598,7 @@ build_index_pathkeys(PlannerInfo *root,
* should stop considering index columns; any lower-order sort
* keys won't be useful either.
*/
- if (!indexcol_is_bool_constant_for_query(index, i))
+ if (!indexcol_is_bool_constant_for_query(root, index, i))
break;
}
diff --git a/src/backend/optimizer/path/tidpath.c b/src/backend/optimizer/path/tidpath.c
index 8ef04060570..0845b460e2c 100644
--- a/src/backend/optimizer/path/tidpath.c
+++ b/src/backend/optimizer/path/tidpath.c
@@ -123,7 +123,7 @@ IsTidEqualClause(RestrictInfo *rinfo, RelOptInfo *rel)
* other side of the clause does.
*/
static bool
-IsTidEqualAnyClause(RestrictInfo *rinfo, RelOptInfo *rel)
+IsTidEqualAnyClause(PlannerInfo *root, RestrictInfo *rinfo, RelOptInfo *rel)
{
ScalarArrayOpExpr *node;
Node *arg1,
@@ -148,7 +148,7 @@ IsTidEqualAnyClause(RestrictInfo *rinfo, RelOptInfo *rel)
IsCTIDVar((Var *) arg1, rel))
{
/* The other argument must be a pseudoconstant */
- if (bms_is_member(rel->relid, pull_varnos(arg2)) ||
+ if (bms_is_member(rel->relid, pull_varnos(root, arg2)) ||
contain_volatile_functions(arg2))
return false;
@@ -190,7 +190,7 @@ IsCurrentOfClause(RestrictInfo *rinfo, RelOptInfo *rel)
* (Using a List may seem a bit weird, but it simplifies the caller.)
*/
static List *
-TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
+TidQualFromRestrictInfo(PlannerInfo *root, RestrictInfo *rinfo, RelOptInfo *rel)
{
/*
* We may ignore pseudoconstant clauses (they can't contain Vars, so could
@@ -210,7 +210,7 @@ TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
* Check all base cases. If we get a match, return the clause.
*/
if (IsTidEqualClause(rinfo, rel) ||
- IsTidEqualAnyClause(rinfo, rel) ||
+ IsTidEqualAnyClause(root, rinfo, rel) ||
IsCurrentOfClause(rinfo, rel))
return list_make1(rinfo);
@@ -227,7 +227,7 @@ TidQualFromRestrictInfo(RestrictInfo *rinfo, RelOptInfo *rel)
* This function is just concerned with handling AND/OR recursion.
*/
static List *
-TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
+TidQualFromRestrictInfoList(PlannerInfo *root, List *rlist, RelOptInfo *rel)
{
List *rlst = NIL;
ListCell *l;
@@ -255,14 +255,14 @@ TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
List *andargs = ((BoolExpr *) orarg)->args;
/* Recurse in case there are sub-ORs */
- sublist = TidQualFromRestrictInfoList(andargs, rel);
+ sublist = TidQualFromRestrictInfoList(root, andargs, rel);
}
else
{
RestrictInfo *rinfo = castNode(RestrictInfo, orarg);
Assert(!restriction_is_or_clause(rinfo));
- sublist = TidQualFromRestrictInfo(rinfo, rel);
+ sublist = TidQualFromRestrictInfo(root, rinfo, rel);
}
/*
@@ -284,7 +284,7 @@ TidQualFromRestrictInfoList(List *rlist, RelOptInfo *rel)
else
{
/* Not an OR clause, so handle base cases */
- rlst = TidQualFromRestrictInfo(rinfo, rel);
+ rlst = TidQualFromRestrictInfo(root, rinfo, rel);
}
/*
@@ -390,7 +390,7 @@ create_tidscan_paths(PlannerInfo *root, RelOptInfo *rel)
* If any suitable quals exist in the rel's baserestrict list, generate a
* plain (unparameterized) TidPath with them.
*/
- tidquals = TidQualFromRestrictInfoList(rel->baserestrictinfo, rel);
+ tidquals = TidQualFromRestrictInfoList(root, rel->baserestrictinfo, rel);
if (tidquals)
{