Skip to content

Commit 2782f3b

Browse files
committed
Revert "Refactor ChangeVarNodesExtended() using the custom callback"
This reverts commit 250a718. It shouldn't be pushed during the release freeze. Reported-by: Tom Lane Discussion: https://postgr.es/m/E1uBIbY-000owH-0O%40gemulon.postgresql.org
1 parent 250a718 commit 2782f3b

File tree

4 files changed

+124
-192
lines changed

4 files changed

+124
-192
lines changed

src/backend/optimizer/plan/analyzejoins.c

Lines changed: 15 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ static bool is_innerrel_unique_for(PlannerInfo *root,
7474
List *restrictlist,
7575
List **extra_clauses);
7676
static int self_join_candidates_cmp(const void *a, const void *b);
77-
static bool replace_relid_callback(Node *node,
78-
ChangeVarNodes_context *context);
7977

8078

8179
/*
@@ -399,8 +397,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
399397
{
400398
Assert(subst > 0);
401399

402-
ChangeVarNodesExtended((Node *) sjinf->semi_rhs_exprs, relid, subst,
403-
0, replace_relid_callback);
400+
ChangeVarNodes((Node *) sjinf->semi_rhs_exprs, relid, subst, 0);
404401
}
405402
}
406403

@@ -472,8 +469,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
472469
sjinfo->ojrelid, subst);
473470
Assert(!bms_is_empty(phv->phrels));
474471

475-
ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0,
476-
replace_relid_callback);
472+
ChangeVarNodes((Node *) phv->phexpr, relid, subst, 0);
477473

478474
Assert(phv->phnullingrels == NULL); /* no need to adjust */
479475
}
@@ -527,8 +523,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
527523
}
528524

529525
if (subst > 0)
530-
ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid,
531-
subst, 0, replace_relid_callback);
526+
ChangeVarNodes((Node *) otherrel->lateral_vars, relid, subst, 0);
532527
}
533528
}
534529

@@ -762,8 +757,7 @@ remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
762757
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
763758

764759
if (sjinfo == NULL)
765-
ChangeVarNodesExtended((Node *) rinfo, relid, subst, 0,
766-
replace_relid_callback);
760+
ChangeVarNodes((Node *) rinfo, relid, subst, 0);
767761
else
768762
remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid);
769763
}
@@ -1554,8 +1548,7 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
15541548
em->em_jdomain->jd_relids = adjust_relid_set(em->em_jdomain->jd_relids, from, to);
15551549

15561550
/* We only process inner joins */
1557-
ChangeVarNodesExtended((Node *) em->em_expr, from, to, 0,
1558-
replace_relid_callback);
1551+
ChangeVarNodes((Node *) em->em_expr, from, to, 0);
15591552

15601553
foreach_node(EquivalenceMember, other, new_members)
15611554
{
@@ -1589,8 +1582,7 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
15891582
continue;
15901583
}
15911584

1592-
ChangeVarNodesExtended((Node *) rinfo, from, to, 0,
1593-
replace_relid_callback);
1585+
ChangeVarNodes((Node *) rinfo, from, to, 0);
15941586

15951587
/*
15961588
* After switching the clause to the remaining relation, check it for
@@ -1685,118 +1677,6 @@ add_non_redundant_clauses(PlannerInfo *root,
16851677
}
16861678
}
16871679

1688-
/*
1689-
* A custom callback for ChangeVarNodesExtended() providing
1690-
* Self-join elimination (SJE) related functionality
1691-
*
1692-
* SJE needs to skip the RangeTblRef node
1693-
* type. During SJE's last step, remove_rel_from_joinlist() removes
1694-
* remaining RangeTblRefs with target relid. If ChangeVarNodes() replaces
1695-
* the target relid before, remove_rel_from_joinlist() fails to identify
1696-
* the nodes to delete.
1697-
*
1698-
* SJE also needs to change the relids within RestrictInfo's.
1699-
*/
1700-
static bool
1701-
replace_relid_callback(Node *node, ChangeVarNodes_context *context)
1702-
{
1703-
if (IsA(node, RangeTblRef))
1704-
{
1705-
return true;
1706-
}
1707-
else if (IsA(node, RestrictInfo))
1708-
{
1709-
RestrictInfo *rinfo = (RestrictInfo *) node;
1710-
int relid = -1;
1711-
bool is_req_equal =
1712-
(rinfo->required_relids == rinfo->clause_relids);
1713-
bool clause_relids_is_multiple =
1714-
(bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
1715-
1716-
/*
1717-
* Recurse down into clauses if the target relation is present in
1718-
* clause_relids or required_relids. We must check required_relids
1719-
* because the relation not present in clause_relids might still be
1720-
* present somewhere in orclause.
1721-
*/
1722-
if (bms_is_member(context->rt_index, rinfo->clause_relids) ||
1723-
bms_is_member(context->rt_index, rinfo->required_relids))
1724-
{
1725-
Relids new_clause_relids;
1726-
1727-
ChangeVarNodesWalkExpression((Node *) rinfo->clause, context);
1728-
ChangeVarNodesWalkExpression((Node *) rinfo->orclause, context);
1729-
1730-
new_clause_relids = adjust_relid_set(rinfo->clause_relids,
1731-
context->rt_index,
1732-
context->new_index);
1733-
1734-
/*
1735-
* Incrementally adjust num_base_rels based on the change of
1736-
* clause_relids, which could contain both base relids and
1737-
* outer-join relids. This operation is legal until we remove
1738-
* only baserels.
1739-
*/
1740-
rinfo->num_base_rels -= bms_num_members(rinfo->clause_relids) -
1741-
bms_num_members(new_clause_relids);
1742-
1743-
rinfo->clause_relids = new_clause_relids;
1744-
rinfo->left_relids =
1745-
adjust_relid_set(rinfo->left_relids, context->rt_index, context->new_index);
1746-
rinfo->right_relids =
1747-
adjust_relid_set(rinfo->right_relids, context->rt_index, context->new_index);
1748-
}
1749-
1750-
if (is_req_equal)
1751-
rinfo->required_relids = rinfo->clause_relids;
1752-
else
1753-
rinfo->required_relids =
1754-
adjust_relid_set(rinfo->required_relids, context->rt_index, context->new_index);
1755-
1756-
rinfo->outer_relids =
1757-
adjust_relid_set(rinfo->outer_relids, context->rt_index, context->new_index);
1758-
rinfo->incompatible_relids =
1759-
adjust_relid_set(rinfo->incompatible_relids, context->rt_index, context->new_index);
1760-
1761-
if (rinfo->mergeopfamilies &&
1762-
bms_get_singleton_member(rinfo->clause_relids, &relid) &&
1763-
clause_relids_is_multiple &&
1764-
relid == context->new_index && IsA(rinfo->clause, OpExpr))
1765-
{
1766-
Expr *leftOp;
1767-
Expr *rightOp;
1768-
1769-
leftOp = (Expr *) get_leftop(rinfo->clause);
1770-
rightOp = (Expr *) get_rightop(rinfo->clause);
1771-
1772-
/*
1773-
* For self-join elimination, changing varnos could transform
1774-
* "t1.a = t2.a" into "t1.a = t1.a". That is always true as long
1775-
* as "t1.a" is not null. We use qual() to check for such a case,
1776-
* and then we replace the qual for a check for not null
1777-
* (NullTest).
1778-
*/
1779-
if (leftOp != NULL && equal(leftOp, rightOp))
1780-
{
1781-
NullTest *ntest = makeNode(NullTest);
1782-
1783-
ntest->arg = leftOp;
1784-
ntest->nulltesttype = IS_NOT_NULL;
1785-
ntest->argisrow = false;
1786-
ntest->location = -1;
1787-
rinfo->clause = (Expr *) ntest;
1788-
rinfo->mergeopfamilies = NIL;
1789-
rinfo->left_em = NULL;
1790-
rinfo->right_em = NULL;
1791-
}
1792-
Assert(rinfo->orclause == NULL);
1793-
}
1794-
return true;
1795-
}
1796-
1797-
return false;
1798-
}
1799-
18001680
/*
18011681
* Remove a relation after we have proven that it participates only in an
18021682
* unneeded unique self-join.
@@ -1845,8 +1725,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
18451725
foreach_node(RestrictInfo, rinfo, joininfos)
18461726
{
18471727
remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
1848-
ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
1849-
0, replace_relid_callback);
1728+
ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
18501729

18511730
if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
18521731
jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1864,8 +1743,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
18641743
restrictlist);
18651744
foreach_node(RestrictInfo, rinfo, toRemove->baserestrictinfo)
18661745
{
1867-
ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid,
1868-
0, replace_relid_callback);
1746+
ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
18691747

18701748
if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
18711749
jinfo_candidates = lappend(jinfo_candidates, rinfo);
@@ -1907,8 +1785,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
19071785
{
19081786
Node *node = lfirst(lc);
19091787

1910-
ChangeVarNodesExtended(node, toRemove->relid, toKeep->relid, 0,
1911-
replace_relid_callback);
1788+
ChangeVarNodes(node, toRemove->relid, toKeep->relid, 0);
19121789
if (!list_member(toKeep->reltarget->exprs, node))
19131790
toKeep->reltarget->exprs = lappend(toKeep->reltarget->exprs, node);
19141791
}
@@ -1952,18 +1829,14 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
19521829
* Replace varno in all the query structures, except nodes RangeTblRef
19531830
* otherwise later remove_rel_from_joinlist will yield errors.
19541831
*/
1955-
ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid,
1956-
0, replace_relid_callback);
1832+
ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid, 0, false);
19571833

19581834
/* Replace links in the planner info */
19591835
remove_rel_from_query(root, toRemove, toKeep->relid, NULL, NULL);
19601836

19611837
/* At last, replace varno in root targetlist and HAVING clause */
1962-
ChangeVarNodesExtended((Node *) root->processed_tlist, toRemove->relid,
1963-
toKeep->relid, 0, replace_relid_callback);
1964-
ChangeVarNodesExtended((Node *) root->processed_groupClause,
1965-
toRemove->relid, toKeep->relid, 0,
1966-
replace_relid_callback);
1838+
ChangeVarNodes((Node *) root->processed_tlist, toRemove->relid, toKeep->relid, 0);
1839+
ChangeVarNodes((Node *) root->processed_groupClause, toRemove->relid, toKeep->relid, 0);
19671840

19681841
adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid);
19691842
adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid);
@@ -2046,10 +1919,8 @@ split_selfjoin_quals(PlannerInfo *root, List *joinquals, List **selfjoinquals,
20461919
* when we have cast of the same var to different (but compatible)
20471920
* types.
20481921
*/
2049-
ChangeVarNodesExtended(rightexpr,
2050-
bms_singleton_member(rinfo->right_relids),
2051-
bms_singleton_member(rinfo->left_relids), 0,
2052-
replace_relid_callback);
1922+
ChangeVarNodes(rightexpr, bms_singleton_member(rinfo->right_relids),
1923+
bms_singleton_member(rinfo->left_relids), 0);
20531924

20541925
if (equal(leftexpr, rightexpr))
20551926
sjoinquals = lappend(sjoinquals, rinfo);
@@ -2085,8 +1956,7 @@ match_unique_clauses(PlannerInfo *root, RelOptInfo *outer, List *uclauses,
20851956
bms_is_empty(rinfo->right_relids));
20861957

20871958
clause = (Expr *) copyObject(rinfo->clause);
2088-
ChangeVarNodesExtended((Node *) clause, relid, outer->relid, 0,
2089-
replace_relid_callback);
1959+
ChangeVarNodes((Node *) clause, relid, outer->relid, 0);
20901960

20911961
iclause = bms_is_empty(rinfo->left_relids) ? get_rightop(clause) :
20921962
get_leftop(clause);

0 commit comments

Comments
 (0)