sharpetronics/PostgreSQL
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-01 00:01:20 -04:00
Code Issues Projects Releases Wiki Activity

Revert "Refactor ChangeVarNodesExtended() using the custom callback"

Browse Source 
This reverts commit 250a718aadad68793e82103282247556a46a3cfc.
It shouldn't be pushed during the release freeze.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/E1uBIbY-000owH-0O%40gemulon.postgresql.org
This commit is contained in:
Alexander Korotkov 2025-05-03 22:42:05 +03:00
parent 250a718aad
commit 2782f3b845
4 changed files with 124 additions and 192 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
src/backend/optimizer/plan/analyzejoins.c
View File

@ -74,8 +74,6 @@ static bool is_innerrel_unique_for(PlannerInfo *root,
List *restrictlist, List *restrictlist,
List **extra_clauses); List **extra_clauses);
static int self_join_candidates_cmp(const void *a, const void *b); static int self_join_candidates_cmp(const void *a, const void *b);
static bool replace_relid_callback(Node *node,
ChangeVarNodes_context *context);
/* /*
@ -399,8 +397,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
{ {
Assert(subst > 0); Assert(subst > 0);
ChangeVarNodesExtended((Node *) sjinf->semi_rhs_exprs, relid, subst, ChangeVarNodes((Node *) sjinf->semi_rhs_exprs, relid, subst, 0);
0, replace_relid_callback);
} }
} }
@ -472,8 +469,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
sjinfo->ojrelid, subst); sjinfo->ojrelid, subst);
Assert(!bms_is_empty(phv->phrels)); Assert(!bms_is_empty(phv->phrels));
ChangeVarNodesExtended((Node *) phv->phexpr, relid, subst, 0, ChangeVarNodes((Node *) phv->phexpr, relid, subst, 0);
replace_relid_callback);
Assert(phv->phnullingrels == NULL); /* no need to adjust */ Assert(phv->phnullingrels == NULL); /* no need to adjust */
} }
@ -527,8 +523,7 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
} }
if (subst > 0) if (subst > 0)
ChangeVarNodesExtended((Node *) otherrel->lateral_vars, relid, ChangeVarNodes((Node *) otherrel->lateral_vars, relid, subst, 0);
subst, 0, replace_relid_callback);
} }
} }
@ -762,8 +757,7 @@ remove_rel_from_eclass(EquivalenceClass *ec, SpecialJoinInfo *sjinfo,
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
if (sjinfo == NULL) if (sjinfo == NULL)
ChangeVarNodesExtended((Node *) rinfo, relid, subst, 0, ChangeVarNodes((Node *) rinfo, relid, subst, 0);
replace_relid_callback);
else else
remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid); remove_rel_from_restrictinfo(rinfo, relid, sjinfo->ojrelid);
} }
@ -1554,8 +1548,7 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
em->em_jdomain->jd_relids = adjust_relid_set(em->em_jdomain->jd_relids, from, to); em->em_jdomain->jd_relids = adjust_relid_set(em->em_jdomain->jd_relids, from, to);
/* We only process inner joins */ /* We only process inner joins */
ChangeVarNodesExtended((Node *) em->em_expr, from, to, 0, ChangeVarNodes((Node *) em->em_expr, from, to, 0);
replace_relid_callback);
foreach_node(EquivalenceMember, other, new_members) foreach_node(EquivalenceMember, other, new_members)
{ {
@ -1589,8 +1582,7 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
continue; continue;
} }
ChangeVarNodesExtended((Node *) rinfo, from, to, 0, ChangeVarNodes((Node *) rinfo, from, to, 0);
replace_relid_callback);
/* /*
* After switching the clause to the remaining relation, check it for * After switching the clause to the remaining relation, check it for
@ -1685,118 +1677,6 @@ add_non_redundant_clauses(PlannerInfo *root,
} }
} }
/*
* A custom callback for ChangeVarNodesExtended() providing
* Self-join elimination (SJE) related functionality
*
* SJE needs to skip the RangeTblRef node
* type. During SJE's last step, remove_rel_from_joinlist() removes
* remaining RangeTblRefs with target relid. If ChangeVarNodes() replaces
* the target relid before, remove_rel_from_joinlist() fails to identify
* the nodes to delete.
*
* SJE also needs to change the relids within RestrictInfo's.
*/
static bool
replace_relid_callback(Node *node, ChangeVarNodes_context *context)
{
if (IsA(node, RangeTblRef))
{
return true;
}
else if (IsA(node, RestrictInfo))
{
RestrictInfo *rinfo = (RestrictInfo *) node;
int relid = -1;
bool is_req_equal =
(rinfo->required_relids == rinfo->clause_relids);
bool clause_relids_is_multiple =
(bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
/*
* Recurse down into clauses if the target relation is present in
* clause_relids or required_relids. We must check required_relids
* because the relation not present in clause_relids might still be
* present somewhere in orclause.
*/
if (bms_is_member(context->rt_index, rinfo->clause_relids) ||
bms_is_member(context->rt_index, rinfo->required_relids))
{
Relids new_clause_relids;
ChangeVarNodesWalkExpression((Node *) rinfo->clause, context);
ChangeVarNodesWalkExpression((Node *) rinfo->orclause, context);
new_clause_relids = adjust_relid_set(rinfo->clause_relids,
context->rt_index,
context->new_index);
/*
* Incrementally adjust num_base_rels based on the change of
* clause_relids, which could contain both base relids and
* outer-join relids. This operation is legal until we remove
* only baserels.
*/
rinfo->num_base_rels -= bms_num_members(rinfo->clause_relids) -
bms_num_members(new_clause_relids);
rinfo->clause_relids = new_clause_relids;
rinfo->left_relids =
adjust_relid_set(rinfo->left_relids, context->rt_index, context->new_index);
rinfo->right_relids =
adjust_relid_set(rinfo->right_relids, context->rt_index, context->new_index);
}
if (is_req_equal)
rinfo->required_relids = rinfo->clause_relids;
else
rinfo->required_relids =
adjust_relid_set(rinfo->required_relids, context->rt_index, context->new_index);
rinfo->outer_relids =
adjust_relid_set(rinfo->outer_relids, context->rt_index, context->new_index);
rinfo->incompatible_relids =
adjust_relid_set(rinfo->incompatible_relids, context->rt_index, context->new_index);
if (rinfo->mergeopfamilies &&
bms_get_singleton_member(rinfo->clause_relids, &relid) &&
clause_relids_is_multiple &&
relid == context->new_index && IsA(rinfo->clause, OpExpr))
{
Expr *leftOp;
Expr *rightOp;
leftOp = (Expr *) get_leftop(rinfo->clause);
rightOp = (Expr *) get_rightop(rinfo->clause);
/*
* For self-join elimination, changing varnos could transform
* "t1.a = t2.a" into "t1.a = t1.a". That is always true as long
* as "t1.a" is not null. We use qual() to check for such a case,
* and then we replace the qual for a check for not null
* (NullTest).
*/
if (leftOp != NULL && equal(leftOp, rightOp))
{
NullTest *ntest = makeNode(NullTest);
ntest->arg = leftOp;
ntest->nulltesttype = IS_NOT_NULL;
ntest->argisrow = false;
ntest->location = -1;
rinfo->clause = (Expr *) ntest;
rinfo->mergeopfamilies = NIL;
rinfo->left_em = NULL;
rinfo->right_em = NULL;
}
Assert(rinfo->orclause == NULL);
}
return true;
}
return false;
}
/* /*
* Remove a relation after we have proven that it participates only in an * Remove a relation after we have proven that it participates only in an
* unneeded unique self-join. * unneeded unique self-join.
@ -1845,8 +1725,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
foreach_node(RestrictInfo, rinfo, joininfos) foreach_node(RestrictInfo, rinfo, joininfos)
{ {
remove_join_clause_from_rels(root, rinfo, rinfo->required_relids); remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid, ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
0, replace_relid_callback);
if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE) if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
jinfo_candidates = lappend(jinfo_candidates, rinfo); jinfo_candidates = lappend(jinfo_candidates, rinfo);
@ -1864,8 +1743,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
restrictlist); restrictlist);
foreach_node(RestrictInfo, rinfo, toRemove->baserestrictinfo) foreach_node(RestrictInfo, rinfo, toRemove->baserestrictinfo)
{ {
ChangeVarNodesExtended((Node *) rinfo, toRemove->relid, toKeep->relid, ChangeVarNodes((Node *) rinfo, toRemove->relid, toKeep->relid, 0);
0, replace_relid_callback);
if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE) if (bms_membership(rinfo->required_relids) == BMS_MULTIPLE)
jinfo_candidates = lappend(jinfo_candidates, rinfo); jinfo_candidates = lappend(jinfo_candidates, rinfo);
@ -1907,8 +1785,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
{ {
Node *node = lfirst(lc); Node *node = lfirst(lc);
ChangeVarNodesExtended(node, toRemove->relid, toKeep->relid, 0, ChangeVarNodes(node, toRemove->relid, toKeep->relid, 0);
replace_relid_callback);
if (!list_member(toKeep->reltarget->exprs, node)) if (!list_member(toKeep->reltarget->exprs, node))
toKeep->reltarget->exprs = lappend(toKeep->reltarget->exprs, node); toKeep->reltarget->exprs = lappend(toKeep->reltarget->exprs, node);
} }
@ -1952,18 +1829,14 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
* Replace varno in all the query structures, except nodes RangeTblRef * Replace varno in all the query structures, except nodes RangeTblRef
* otherwise later remove_rel_from_joinlist will yield errors. * otherwise later remove_rel_from_joinlist will yield errors.
*/ */
ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid, ChangeVarNodesExtended((Node *) root->parse, toRemove->relid, toKeep->relid, 0, false);
0, replace_relid_callback);
/* Replace links in the planner info */ /* Replace links in the planner info */
remove_rel_from_query(root, toRemove, toKeep->relid, NULL, NULL); remove_rel_from_query(root, toRemove, toKeep->relid, NULL, NULL);
/* At last, replace varno in root targetlist and HAVING clause */ /* At last, replace varno in root targetlist and HAVING clause */
ChangeVarNodesExtended((Node *) root->processed_tlist, toRemove->relid, ChangeVarNodes((Node *) root->processed_tlist, toRemove->relid, toKeep->relid, 0);
toKeep->relid, 0, replace_relid_callback); ChangeVarNodes((Node *) root->processed_groupClause, toRemove->relid, toKeep->relid, 0);
ChangeVarNodesExtended((Node *) root->processed_groupClause,
toRemove->relid, toKeep->relid, 0,
replace_relid_callback);
adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid); adjust_relid_set(root->all_result_relids, toRemove->relid, toKeep->relid);
adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid); adjust_relid_set(root->leaf_result_relids, toRemove->relid, toKeep->relid);
@ -2046,10 +1919,8 @@ split_selfjoin_quals(PlannerInfo *root, List *joinquals, List **selfjoinquals,
* when we have cast of the same var to different (but compatible) * when we have cast of the same var to different (but compatible)
* types. * types.
*/ */
ChangeVarNodesExtended(rightexpr, ChangeVarNodes(rightexpr, bms_singleton_member(rinfo->right_relids),
bms_singleton_member(rinfo->right_relids), bms_singleton_member(rinfo->left_relids), 0);
bms_singleton_member(rinfo->left_relids), 0,
replace_relid_callback);
if (equal(leftexpr, rightexpr)) if (equal(leftexpr, rightexpr))
sjoinquals = lappend(sjoinquals, rinfo); sjoinquals = lappend(sjoinquals, rinfo);
@ -2085,8 +1956,7 @@ match_unique_clauses(PlannerInfo *root, RelOptInfo *outer, List *uclauses,
bms_is_empty(rinfo->right_relids)); bms_is_empty(rinfo->right_relids));
clause = (Expr *) copyObject(rinfo->clause); clause = (Expr *) copyObject(rinfo->clause);
ChangeVarNodesExtended((Node *) clause, relid, outer->relid, 0, ChangeVarNodes((Node *) clause, relid, outer->relid, 0);
replace_relid_callback);
iclause = bms_is_empty(rinfo->left_relids) ? get_rightop(clause) : iclause = bms_is_empty(rinfo->left_relids) ? get_rightop(clause) :
get_leftop(clause); get_leftop(clause);

138
src/backend/rewrite/rewriteManip.c
View File

@ -550,15 +550,19 @@ offset_relid_set(Relids relids, int offset)
* earlier to ensure that no unwanted side-effects occur! * earlier to ensure that no unwanted side-effects occur!
*/ */
typedef struct
{
int rt_index;
int new_index;
int sublevels_up;
bool change_RangeTblRef;
} ChangeVarNodes_context;
static bool static bool
ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context) ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
{ {
if (node == NULL) if (node == NULL)
return false; return false;
if (context->callback && context->callback(node, context))
return false;
if (IsA(node, Var)) if (IsA(node, Var))
{ {
Var *var = (Var *) node; Var *var = (Var *) node;
@ -584,7 +588,7 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
cexpr->cvarno = context->new_index; cexpr->cvarno = context->new_index;
return false; return false;
} }
if (IsA(node, RangeTblRef)) if (IsA(node, RangeTblRef) && context->change_RangeTblRef)
{ {
RangeTblRef *rtr = (RangeTblRef *) node; RangeTblRef *rtr = (RangeTblRef *) node;
@ -631,6 +635,95 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
} }
return false; return false;
} }
if (IsA(node, RestrictInfo))
{
RestrictInfo *rinfo = (RestrictInfo *) node;
int relid = -1;
bool is_req_equal =
(rinfo->required_relids == rinfo->clause_relids);
bool clause_relids_is_multiple =
(bms_membership(rinfo->clause_relids) == BMS_MULTIPLE);
/*
* Recurse down into clauses if the target relation is present in
* clause_relids or required_relids. We must check required_relids
* because the relation not present in clause_relids might still be
* present somewhere in orclause.
*/
if (bms_is_member(context->rt_index, rinfo->clause_relids) ||
bms_is_member(context->rt_index, rinfo->required_relids))
{
Relids new_clause_relids;
expression_tree_walker((Node *) rinfo->clause, ChangeVarNodes_walker, (void *) context);
expression_tree_walker((Node *) rinfo->orclause, ChangeVarNodes_walker, (void *) context);
new_clause_relids = adjust_relid_set(rinfo->clause_relids,
context->rt_index,
context->new_index);
/*
* Incrementally adjust num_base_rels based on the change of
* clause_relids, which could contain both base relids and
* outer-join relids. This operation is legal until we remove
* only baserels.
*/
rinfo->num_base_rels -= bms_num_members(rinfo->clause_relids) -
bms_num_members(new_clause_relids);
rinfo->clause_relids = new_clause_relids;
rinfo->left_relids =
adjust_relid_set(rinfo->left_relids, context->rt_index, context->new_index);
rinfo->right_relids =
adjust_relid_set(rinfo->right_relids, context->rt_index, context->new_index);
}
if (is_req_equal)
rinfo->required_relids = rinfo->clause_relids;
else
rinfo->required_relids =
adjust_relid_set(rinfo->required_relids, context->rt_index, context->new_index);
rinfo->outer_relids =
adjust_relid_set(rinfo->outer_relids, context->rt_index, context->new_index);
rinfo->incompatible_relids =
adjust_relid_set(rinfo->incompatible_relids, context->rt_index, context->new_index);
if (rinfo->mergeopfamilies &&
bms_get_singleton_member(rinfo->clause_relids, &relid) &&
clause_relids_is_multiple &&
relid == context->new_index && IsA(rinfo->clause, OpExpr))
{
Expr *leftOp;
Expr *rightOp;
leftOp = (Expr *) get_leftop(rinfo->clause);
rightOp = (Expr *) get_rightop(rinfo->clause);
/*
* For self-join elimination, changing varnos could transform
* "t1.a = t2.a" into "t1.a = t1.a". That is always true as long
* as "t1.a" is not null. We use qual() to check for such a case,
* and then we replace the qual for a check for not null
* (NullTest).
*/
if (leftOp != NULL && equal(leftOp, rightOp))
{
NullTest *ntest = makeNode(NullTest);
ntest->arg = leftOp;
ntest->nulltesttype = IS_NOT_NULL;
ntest->argisrow = false;
ntest->location = -1;
rinfo->clause = (Expr *) ntest;
rinfo->mergeopfamilies = NIL;
rinfo->left_em = NULL;
rinfo->right_em = NULL;
}
Assert(rinfo->orclause == NULL);
}
return false;
}
if (IsA(node, AppendRelInfo)) if (IsA(node, AppendRelInfo))
{ {
AppendRelInfo *appinfo = (AppendRelInfo *) node; AppendRelInfo *appinfo = (AppendRelInfo *) node;
@ -664,28 +757,26 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
} }
/* /*
* ChangeVarNodesExtended - similar to ChangeVarNodes, but with an additional * ChangeVarNodesExtended - similar to ChangeVarNodes, but has additional
* 'callback' param * 'change_RangeTblRef' param
* *
* ChangeVarNodes changes a given node and all of its underlying nodes. * ChangeVarNodes changes a given node and all of its underlying nodes.
* This version of function additionally takes a callback, which has a * However, self-join elimination (SJE) needs to skip the RangeTblRef node
* chance to process a node before ChangeVarNodes_walker. A callback * type. During SJE's last step, remove_rel_from_joinlist() removes
* returns a boolean value indicating if given node should be skipped from * remaining RangeTblRefs with target relid. If ChangeVarNodes() replaces
* further processing by ChangeVarNodes_walker. The callback is called * the target relid before, remove_rel_from_joinlist() fails to identify
* only for expressions and other children nodes of a Query processed by * the nodes to delete.
* a walker. Initial processing of the root Query doesn't involve the
* callback.
*/ */
void void
ChangeVarNodesExtended(Node *node, int rt_index, int new_index, ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
int sublevels_up, ChangeVarNodes_callback callback) int sublevels_up, bool change_RangeTblRef)
{ {
ChangeVarNodes_context context; ChangeVarNodes_context context;
context.rt_index = rt_index; context.rt_index = rt_index;
context.new_index = new_index; context.new_index = new_index;
context.sublevels_up = sublevels_up; context.sublevels_up = sublevels_up;
context.callback = callback; context.change_RangeTblRef = change_RangeTblRef;
/* /*
* Must be prepared to start with a Query or a bare expression tree; if * Must be prepared to start with a Query or a bare expression tree; if
@ -735,20 +826,7 @@ ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
void void
ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up) ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
{ {
ChangeVarNodesExtended(node, rt_index, new_index, sublevels_up, NULL); ChangeVarNodesExtended(node, rt_index, new_index, sublevels_up, true);
}
/*
* ChangeVarNodesWalkExpression - process expression within the custom
* callback provided to the
* ChangeVarNodesExtended.
*/
bool
ChangeVarNodesWalkExpression(Node *node, ChangeVarNodes_context *context)
{
return expression_tree_walker(node,
ChangeVarNodes_walker,
(void *) context);
} }
/* /*

17
src/include/rewrite/rewriteManip.h
View File

@ -41,18 +41,6 @@ typedef enum ReplaceVarsNoMatchOption
REPLACEVARS_SUBSTITUTE_NULL, /* replace with a NULL Const */ REPLACEVARS_SUBSTITUTE_NULL, /* replace with a NULL Const */
} ReplaceVarsNoMatchOption; } ReplaceVarsNoMatchOption;
typedef struct ChangeVarNodes_context ChangeVarNodes_context;
typedef bool (*ChangeVarNodes_callback) (Node *node,
ChangeVarNodes_context *arg);
struct ChangeVarNodes_context
{
int rt_index;
int new_index;
int sublevels_up;
ChangeVarNodes_callback callback;
};
extern Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid); extern Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid);
extern void CombineRangeTables(List **dst_rtable, List **dst_perminfos, extern void CombineRangeTables(List **dst_rtable, List **dst_perminfos,
@ -61,10 +49,7 @@ extern void OffsetVarNodes(Node *node, int offset, int sublevels_up);
extern void ChangeVarNodes(Node *node, int rt_index, int new_index, extern void ChangeVarNodes(Node *node, int rt_index, int new_index,
int sublevels_up); int sublevels_up);
extern void ChangeVarNodesExtended(Node *node, int rt_index, int new_index, extern void ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
int sublevels_up, int sublevels_up, bool change_RangeTblRef);
ChangeVarNodes_callback callback);
extern bool ChangeVarNodesWalkExpression(Node *node,
ChangeVarNodes_context *context);
extern void IncrementVarSublevelsUp(Node *node, int delta_sublevels_up, extern void IncrementVarSublevelsUp(Node *node, int delta_sublevels_up,
int min_sublevels_up); int min_sublevels_up);
extern void IncrementVarSublevelsUp_rtable(List *rtable, extern void IncrementVarSublevelsUp_rtable(List *rtable,

1
src/tools/pgindent/typedefs.list
View File

@ -410,7 +410,6 @@ CatCacheHeader
CatalogId CatalogId
CatalogIdMapEntry CatalogIdMapEntry
CatalogIndexState CatalogIndexState
ChangeVarNodes_callback
ChangeVarNodes_context ChangeVarNodes_context
CheckPoint CheckPoint
CheckPointStmt CheckPointStmt