Re: BUG #17768: Assert triggered on initsplan.c - Mailing list pgsql-bugs
| From | Tom Lane |
|---|---|
| Subject | Re: BUG #17768: Assert triggered on initsplan.c |
| Date | |
| Msg-id | 1819212.1675463481@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: BUG #17768: Assert triggered on initsplan.c (Tom Lane <tgl@sss.pgh.pa.us>) |
| List | pgsql-bugs |
I wrote:
> I've not looked closely at this, but ... I remember thinking while
> revising deconstruct_jointree that the whole PostponedQual mechanism
> was a wart we should try to get rid of. I didn't touch it since
> I saw no obvious bugs and the patch was too large already, but maybe
> now is the time to try harder.
Here's a draft patch (no test cases as yet) that does it like that.
I think this is morally equivalent to what you did, but perhaps a
bit neater and faster.
regards, tom lane
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 35b2dc1034..d61b827f02 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -62,6 +62,8 @@ typedef struct JoinTreeItem
/* Fields filled during deconstruct_recurse: */
Node *jtnode; /* jointree node to examine */
JoinDomain *jdomain; /* join domain for its ON/WHERE clauses */
+ struct JoinTreeItem *jti_parent; /* JoinTreeItem for this node's
+ * parent, or NULL if it's the top */
Relids qualscope; /* base+OJ Relids syntactically included in
* this jointree node */
Relids inner_join_rels; /* base+OJ Relids syntactically included
@@ -74,26 +76,20 @@ typedef struct JoinTreeItem
/* Fields filled during deconstruct_distribute: */
SpecialJoinInfo *sjinfo; /* if outer join, its SpecialJoinInfo */
List *oj_joinclauses; /* outer join quals not yet distributed */
+ List *lateral_clauses; /* quals postponed from children due to
+ * lateral references */
} JoinTreeItem;
-/* Elements of the postponed_qual_list used during deconstruct_distribute */
-typedef struct PostponedQual
-{
- Node *qual; /* a qual clause waiting to be processed */
- Relids relids; /* the set of baserels it references */
-} PostponedQual;
-
static void extract_lateral_references(PlannerInfo *root, RelOptInfo *brel,
Index rtindex);
static List *deconstruct_recurse(PlannerInfo *root, Node *jtnode,
JoinDomain *parent_domain,
+ JoinTreeItem *parent_jtitem,
List **item_list);
-static void deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem,
- List **postponed_qual_list);
+static void deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem);
static void process_security_barrier_quals(PlannerInfo *root,
- int rti, Relids qualscope,
- JoinDomain *jdomain);
+ int rti, JoinTreeItem *jtitem);
static void mark_rels_nulled_by_join(PlannerInfo *root, Index ojrelid,
Relids lower_rels);
static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
@@ -107,7 +103,7 @@ static void deconstruct_distribute_oj_quals(PlannerInfo *root,
List *jtitems,
JoinTreeItem *jtitem);
static void distribute_quals_to_rels(PlannerInfo *root, List *clauses,
- JoinDomain *jdomain,
+ JoinTreeItem *jtitem,
SpecialJoinInfo *sjinfo,
Index security_level,
Relids qualscope,
@@ -116,10 +112,9 @@ static void distribute_quals_to_rels(PlannerInfo *root, List *clauses,
bool allow_equivalence,
bool has_clone,
bool is_clone,
- List **postponed_qual_list,
List **postponed_oj_qual_list);
static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
- JoinDomain *jdomain,
+ JoinTreeItem *jtitem,
SpecialJoinInfo *sjinfo,
Index security_level,
Relids qualscope,
@@ -128,7 +123,6 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
bool allow_equivalence,
bool has_clone,
bool is_clone,
- List **postponed_qual_list,
List **postponed_oj_qual_list);
static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
static void check_mergejoinable(RestrictInfo *restrictinfo);
@@ -742,7 +736,6 @@ deconstruct_jointree(PlannerInfo *root)
List *result;
JoinDomain *top_jdomain;
List *item_list = NIL;
- List *postponed_qual_list = NIL;
ListCell *lc;
/*
@@ -766,7 +759,7 @@ deconstruct_jointree(PlannerInfo *root)
/* Perform the initial scan of the jointree */
result = deconstruct_recurse(root, (Node *) root->parse->jointree,
- top_jdomain,
+ top_jdomain, NULL,
&item_list);
/* Now we can form the value of all_query_rels, too */
@@ -780,16 +773,12 @@ deconstruct_jointree(PlannerInfo *root)
{
JoinTreeItem *jtitem = (JoinTreeItem *) lfirst(lc);
- deconstruct_distribute(root, jtitem,
- &postponed_qual_list);
+ deconstruct_distribute(root, jtitem);
}
- /* Shouldn't be any leftover postponed quals */
- Assert(postponed_qual_list == NIL);
-
/*
- * However, if there were any special joins then we may have some
- * postponed LEFT JOIN clauses to deal with.
+ * If there were any special joins then we may have some postponed LEFT
+ * JOIN clauses to deal with.
*/
if (root->join_info_list)
{
@@ -814,7 +803,8 @@ deconstruct_jointree(PlannerInfo *root)
*
* jtnode is the jointree node to examine, and parent_domain is the
* enclosing join domain. (We must add all base+OJ relids appearing
- * here or below to parent_domain.)
+ * here or below to parent_domain.) parent_jtitem is the JoinTreeItem
+ * for the parent jointree node, or NULL at the top of the recursion.
*
* item_list is an in/out parameter: we add a JoinTreeItem struct to
* that list for each jointree node, in depth-first traversal order.
@@ -825,6 +815,7 @@ deconstruct_jointree(PlannerInfo *root)
static List *
deconstruct_recurse(PlannerInfo *root, Node *jtnode,
JoinDomain *parent_domain,
+ JoinTreeItem *parent_jtitem,
List **item_list)
{
List *joinlist;
@@ -835,6 +826,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
/* Make the new JoinTreeItem, but don't add it to item_list yet */
jtitem = palloc0_object(JoinTreeItem);
jtitem->jtnode = jtnode;
+ jtitem->jti_parent = parent_jtitem;
if (IsA(jtnode, RangeTblRef))
{
@@ -880,6 +872,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
sub_joinlist = deconstruct_recurse(root, lfirst(l),
parent_domain,
+ jtitem,
item_list);
sub_item = (JoinTreeItem *) llast(*item_list);
jtitem->qualscope = bms_add_members(jtitem->qualscope,
@@ -922,10 +915,12 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
/* Recurse */
leftjoinlist = deconstruct_recurse(root, j->larg,
parent_domain,
+ jtitem,
item_list);
left_item = (JoinTreeItem *) llast(*item_list);
rightjoinlist = deconstruct_recurse(root, j->rarg,
parent_domain,
+ jtitem,
item_list);
right_item = (JoinTreeItem *) llast(*item_list);
/* Compute qualscope etc */
@@ -947,10 +942,12 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
/* Recurse */
leftjoinlist = deconstruct_recurse(root, j->larg,
parent_domain,
+ jtitem,
item_list);
left_item = (JoinTreeItem *) llast(*item_list);
rightjoinlist = deconstruct_recurse(root, j->rarg,
child_domain,
+ jtitem,
item_list);
right_item = (JoinTreeItem *) llast(*item_list);
/* Compute join domain contents, qualscope etc */
@@ -984,10 +981,12 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
/* Recurse */
leftjoinlist = deconstruct_recurse(root, j->larg,
parent_domain,
+ jtitem,
item_list);
left_item = (JoinTreeItem *) llast(*item_list);
rightjoinlist = deconstruct_recurse(root, j->rarg,
parent_domain,
+ jtitem,
item_list);
right_item = (JoinTreeItem *) llast(*item_list);
/* Compute qualscope etc */
@@ -1013,6 +1012,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
root->join_domains = lappend(root->join_domains, child_domain);
leftjoinlist = deconstruct_recurse(root, j->larg,
child_domain,
+ jtitem,
item_list);
left_item = (JoinTreeItem *) llast(*item_list);
fj_domain->jd_relids = bms_copy(child_domain->jd_relids);
@@ -1021,6 +1021,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
root->join_domains = lappend(root->join_domains, child_domain);
rightjoinlist = deconstruct_recurse(root, j->rarg,
child_domain,
+ jtitem,
item_list);
right_item = (JoinTreeItem *) llast(*item_list);
/* Compute qualscope etc */
@@ -1108,20 +1109,9 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode,
*
* Distribute quals of the node to appropriate restriction and join lists.
* In addition, entries will be added to root->join_info_list for outer joins.
- *
- * Inputs:
- * jtitem is the JoinTreeItem to examine
- * Input/Outputs:
- * *postponed_qual_list is a list of PostponedQual structs
- *
- * On entry, *postponed_qual_list contains any quals that had to be postponed
- * out of lower join levels (because they contain lateral references).
- * On exit, *postponed_qual_list contains quals that can't be processed yet
- * (because their lateral references are still unsatisfied).
*/
static void
-deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem,
- List **postponed_qual_list)
+deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem)
{
Node *jtnode = jtitem->jtnode;
@@ -1133,82 +1123,51 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem,
if (root->qual_security_level > 0)
process_security_barrier_quals(root,
varno,
- jtitem->qualscope,
- jtitem->jdomain);
+ jtitem);
}
else if (IsA(jtnode, FromExpr))
{
FromExpr *f = (FromExpr *) jtnode;
- List *new_postponed_quals = NIL;
- ListCell *l;
/*
- * Try to process any quals postponed by children. If they need
- * further postponement, add them to my output postponed_qual_list.
+ * Process any lateral-referencing quals that were postponed to this
+ * level by children.
*/
- foreach(l, *postponed_qual_list)
- {
- PostponedQual *pq = (PostponedQual *) lfirst(l);
-
- if (bms_is_subset(pq->relids, jtitem->qualscope))
- distribute_qual_to_rels(root, pq->qual,
- jtitem->jdomain,
- NULL,
- root->qual_security_level,
- jtitem->qualscope, NULL, NULL,
- true, false, false,
- NULL, NULL);
- else
- new_postponed_quals = lappend(new_postponed_quals, pq);
- }
- *postponed_qual_list = new_postponed_quals;
+ distribute_quals_to_rels(root, jtitem->lateral_clauses,
+ jtitem,
+ NULL,
+ root->qual_security_level,
+ jtitem->qualscope, NULL, NULL,
+ true, false, false,
+ NULL);
/*
* Now process the top-level quals.
*/
distribute_quals_to_rels(root, (List *) f->quals,
- jtitem->jdomain,
+ jtitem,
NULL,
root->qual_security_level,
jtitem->qualscope, NULL, NULL,
true, false, false,
- postponed_qual_list, NULL);
+ NULL);
}
else if (IsA(jtnode, JoinExpr))
{
JoinExpr *j = (JoinExpr *) jtnode;
- List *new_postponed_quals = NIL;
Relids ojscope;
List *my_quals;
SpecialJoinInfo *sjinfo;
List **postponed_oj_qual_list;
- ListCell *l;
/*
- * Try to process any quals postponed by children. If they need
- * further postponement, add them to my output postponed_qual_list.
- * Quals that can be processed now must be included in my_quals, so
- * that they'll be handled properly in make_outerjoininfo.
+ * Include lateral-referencing quals postponed from children in
+ * my_quals, so that they'll be handled properly in
+ * make_outerjoininfo. (This is destructive to
+ * jtitem->lateral_clauses, but we won't use that again.)
*/
- my_quals = NIL;
- foreach(l, *postponed_qual_list)
- {
- PostponedQual *pq = (PostponedQual *) lfirst(l);
-
- if (bms_is_subset(pq->relids, jtitem->qualscope))
- my_quals = lappend(my_quals, pq->qual);
- else
- {
- /*
- * We should not be postponing any quals past an outer join.
- * If this Assert fires, pull_up_subqueries() messed up.
- */
- Assert(j->jointype == JOIN_INNER);
- new_postponed_quals = lappend(new_postponed_quals, pq);
- }
- }
- *postponed_qual_list = new_postponed_quals;
- my_quals = list_concat(my_quals, (List *) j->quals);
+ my_quals = list_concat(jtitem->lateral_clauses,
+ (List *) j->quals);
/*
* For an OJ, form the SpecialJoinInfo now, so that we can pass it to
@@ -1268,14 +1227,13 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem,
/* Process the JOIN's qual clauses */
distribute_quals_to_rels(root, my_quals,
- jtitem->jdomain,
+ jtitem,
sjinfo,
root->qual_security_level,
jtitem->qualscope,
ojscope, jtitem->nonnullable_rels,
true, /* allow_equivalence */
false, false, /* not clones */
- postponed_qual_list,
postponed_oj_qual_list);
/* And add the SpecialJoinInfo to join_info_list */
@@ -1304,8 +1262,7 @@ deconstruct_distribute(PlannerInfo *root, JoinTreeItem *jtitem,
*/
static void
process_security_barrier_quals(PlannerInfo *root,
- int rti, Relids qualscope,
- JoinDomain *jdomain)
+ int rti, JoinTreeItem *jtitem)
{
RangeTblEntry *rte = root->simple_rte_array[rti];
Index security_level = 0;
@@ -1328,15 +1285,14 @@ process_security_barrier_quals(PlannerInfo *root,
* pushed up to top of tree, which we don't want.
*/
distribute_quals_to_rels(root, qualset,
- jdomain,
+ jtitem,
NULL,
security_level,
- qualscope,
- qualscope,
+ jtitem->qualscope,
+ jtitem->qualscope,
NULL,
true,
false, false, /* not clones */
- NULL,
NULL);
security_level++;
}
@@ -2038,7 +1994,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
is_clone = !has_clone;
distribute_quals_to_rels(root, quals,
- otherjtitem->jdomain,
+ otherjtitem,
sjinfo,
root->qual_security_level,
this_qualscope,
@@ -2046,7 +2002,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
allow_equivalence,
has_clone,
is_clone,
- NULL, NULL); /* no more postponement */
+ NULL); /* no more postponement */
/*
* Adjust qual nulling bits for next level up, if needed. We
@@ -2067,14 +2023,14 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
{
/* No commutation possible, just process the postponed clauses */
distribute_quals_to_rels(root, jtitem->oj_joinclauses,
- jtitem->jdomain,
+ jtitem,
sjinfo,
root->qual_security_level,
qualscope,
ojscope, nonnullable_rels,
true, /* allow_equivalence */
false, false, /* not clones */
- NULL, NULL); /* no more postponement */
+ NULL); /* no more postponement */
}
}
@@ -2092,7 +2048,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
*/
static void
distribute_quals_to_rels(PlannerInfo *root, List *clauses,
- JoinDomain *jdomain,
+ JoinTreeItem *jtitem,
SpecialJoinInfo *sjinfo,
Index security_level,
Relids qualscope,
@@ -2101,7 +2057,6 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
bool allow_equivalence,
bool has_clone,
bool is_clone,
- List **postponed_qual_list,
List **postponed_oj_qual_list)
{
ListCell *lc;
@@ -2111,7 +2066,7 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
Node *clause = (Node *) lfirst(lc);
distribute_qual_to_rels(root, clause,
- jdomain,
+ jtitem,
sjinfo,
security_level,
qualscope,
@@ -2120,7 +2075,6 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
allow_equivalence,
has_clone,
is_clone,
- postponed_qual_list,
postponed_oj_qual_list);
}
}
@@ -2134,12 +2088,12 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
* mergejoinable operator, enter its left- and right-side expressions into
* the query's EquivalenceClasses.
*
- * In some cases, quals will be added to postponed_qual_list or
+ * In some cases, quals will be added to parent jtitems' lateral_clauses or
* postponed_oj_qual_list instead of being processed right away.
* These will be dealt with in later steps of deconstruct_jointree.
*
* 'clause': the qual clause to be distributed
- * 'jdomain': the join domain containing the clause
+ * 'jtitem': the JoinTreeItem for the containing jointree node
* 'sjinfo': join's SpecialJoinInfo (NULL for an inner join or WHERE clause)
* 'security_level': security_level to assign to the qual
* 'qualscope': set of base+OJ rels the qual's syntactic scope covers
@@ -2153,9 +2107,6 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
* EquivalenceClass
* 'has_clone': has_clone property to assign to the qual
* 'is_clone': is_clone property to assign to the qual
- * 'postponed_qual_list': list of PostponedQual structs, which we can add
- * this qual to if it turns out to belong to a higher join level.
- * Can be NULL if caller knows postponement is impossible.
* 'postponed_oj_qual_list': if not NULL, non-degenerate outer join clauses
* should be added to this list instead of being processed (list entries
* are just the bare clauses)
@@ -2170,7 +2121,7 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses,
*/
static void
distribute_qual_to_rels(PlannerInfo *root, Node *clause,
- JoinDomain *jdomain,
+ JoinTreeItem *jtitem,
SpecialJoinInfo *sjinfo,
Index security_level,
Relids qualscope,
@@ -2179,7 +2130,6 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
bool allow_equivalence,
bool has_clone,
bool is_clone,
- List **postponed_qual_list,
List **postponed_oj_qual_list)
{
Relids relids;
@@ -2202,19 +2152,32 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
* level that includes every rel they reference. Although we could make
* pull_up_subqueries() place such quals correctly to begin with, it's
* easier to handle it here. When we find a clause that contains Vars
- * outside its syntactic scope, we add it to the postponed-quals list, and
- * process it once we've recursed back up to the appropriate join level.
+ * outside its syntactic scope, locate the nearest parent join level that
+ * includes all the required rels and add the clause to that level's
+ * lateral_clauses list. We'll process it when we reach that join level.
*/
if (!bms_is_subset(relids, qualscope))
{
- PostponedQual *pq = (PostponedQual *) palloc(sizeof(PostponedQual));
+ JoinTreeItem *pitem;
Assert(root->hasLateralRTEs); /* shouldn't happen otherwise */
Assert(sjinfo == NULL); /* mustn't postpone past outer join */
- pq->qual = clause;
- pq->relids = relids;
- *postponed_qual_list = lappend(*postponed_qual_list, pq);
- return;
+ for (pitem = jtitem->jti_parent; pitem; pitem = pitem->jti_parent)
+ {
+ if (bms_is_subset(relids, pitem->qualscope))
+ {
+ pitem->lateral_clauses = lappend(pitem->lateral_clauses,
+ clause);
+ return;
+ }
+
+ /*
+ * We should not be postponing any quals past an outer join. If
+ * this Assert fires, pull_up_subqueries() messed up.
+ */
+ Assert(pitem->sjinfo == NULL);
+ }
+ elog(ERROR, "failed to postpone qual containing lateral reference");
}
/*
@@ -2262,7 +2225,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
else
{
/* eval at join domain level */
- relids = bms_copy(jdomain->jd_relids);
+ relids = bms_copy(jtitem->jdomain->jd_relids);
/* mark as gating qual */
pseudoconstant = true;
/* tell createplan.c to check for gating quals */
@@ -2458,7 +2421,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
{
if (maybe_equivalence)
{
- if (process_equivalence(root, &restrictinfo, jdomain))
+ if (process_equivalence(root, &restrictinfo, jtitem->jdomain))
return;
/* EC rejected it, so set left_ec/right_ec the hard way ... */
if (restrictinfo->mergeopfamilies) /* EC might have changed this */
pgsql-bugs by date: