rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Rethink treatment of "postponed" quals in deconstruct_jointree(). 

After pulling up LATERAL subqueries, we may have qual clauses that
refer to relations outside their syntactic scope.  Before doing any
such pullup, prepjointree.c checks to make sure that it wouldn't
create a semantically-invalid situation; but we leave it to
deconstruct_jointree() to actually move these quals up the join
tree to a place where they can be evaluated.  In commit 2489d76c4,
I (tgl) refactored deconstruct_jointree() in a way that caused
assertion failures while moving such quals, because the new logic
failed to distinguish "this jointree node is a parent of the source
one" from "this jointree node is processed after the source
one in depth-first order".

Fix this, and at the same time reduce the overhead a bit, by
getting rid of the common PostponedQual list and instead making each
JoinTreeItem contain a list of quals that needed to be postponed to
its level.  We can help distribute_qual_to_rels find the appropriate
JoinTreeItem efficiently by adding parent-item links to the
JoinTreeItem data structure.  This ends up being the same number
of relid subset checks as the original (pre-bug) logic, but less
list manipulation is required during multi-level postponements.

Richard Guo and Tom Lane, per bug #17768 from Robins Tharakan.

Discussion: https://postgr.es/m/17768-5ac8730ece54478f@postgresql.org
This commit is contained in:
Tom Lane 2023-02-04 12:45:53 -05:00
parent faff8f8e47
commit 5840c20272
3 changed files with 111 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

203
src/backend/optimizer/plan/initsplan.c
View File

@ -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
* postponed_oj_qual_list instead of being processed right away.
* These will be dealt with in later steps of deconstruct_jointree.
* In some cases, quals will be added to parent jtitems' lateral_clauses
* or to postponed_oj_qual_list instead of being processed right away.
* These will be dealt with in later calls of deconstruct_distribute.
*
* '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 */

22
src/test/regress/expected/join.out
View File

@ -6203,6 +6203,28 @@ select * from
Output: 3
(11 rows)
-- a new postponed-quals issue (bug #17768)
explain (costs off)
select * from int4_tbl t1,
lateral (select * from int4_tbl t2 inner join int4_tbl t3 on t1.f1 = 1
inner join (int4_tbl t4 left join int4_tbl t5 on true) on true) ss;
QUERY PLAN
-------------------------------------------------
Nested Loop Left Join
-> Nested Loop
-> Nested Loop
-> Nested Loop
-> Seq Scan on int4_tbl t1
Filter: (f1 = 1)
-> Seq Scan on int4_tbl t2
-> Materialize
-> Seq Scan on int4_tbl t3
-> Materialize
-> Seq Scan on int4_tbl t4
-> Materialize
-> Seq Scan on int4_tbl t5
(13 rows)
-- check dummy rels with lateral references (bug #15694)
explain (verbose, costs off)
select * from int8_tbl i8 left join lateral

6
src/test/regress/sql/join.sql
View File

@ -2159,6 +2159,12 @@ select * from
select * from (select 3 as z offset 0) z where z.z = x.x
) zz on zz.z = y.y;
-- a new postponed-quals issue (bug #17768)
explain (costs off)
select * from int4_tbl t1,
lateral (select * from int4_tbl t2 inner join int4_tbl t3 on t1.f1 = 1
inner join (int4_tbl t4 left join int4_tbl t5 on true) on true) ss;
-- check dummy rels with lateral references (bug #15694)
explain (verbose, costs off)
select * from int8_tbl i8 left join lateral