Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit add673b

Browse files
committed
Fix Asserts in calc_non_nestloop_required_outer().
These were not testing the same thing as the comparable Assert in calc_nestloop_required_outer(), because we neglected to map the given Paths' relids to top-level relids. When considering a partition child join the latter is the correct thing to do. This oversight is old, but since it's only an overly-weak Assert check there doesn't seem to be much value in back-patching. Richard Guo (with cosmetic changes and comment updates by me) Discussion: https://postgr.es/m/CAMbWs49sqbe9GBZ8sy8dSfKRNURgicR85HX8vgzcgQsPF0XY1w@mail.gmail.com
1 parent d641b82 commit add673b

File tree

2 files changed

+29
-4
lines changed

2 files changed

+29
-4
lines changed

src/backend/optimizer/path/joinpath.c

+5-2
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,11 @@ try_nestloop_path(PlannerInfo *root,
730730
return;
731731

732732
/*
733-
* Paths are parameterized by top-level parents, so run parameterization
734-
* tests on the parent relids.
733+
* Any parameterization of the input paths refers to topmost parents of
734+
* the relevant relations, because reparameterize_path_by_child() hasn't
735+
* been called yet. So we must consider topmost parents of the relations
736+
* being joined, too, while determining parameterization of the result and
737+
* checking for disallowed parameterization cases.
735738
*/
736739
if (innerrel->top_parent_relids)
737740
innerrelids = innerrel->top_parent_relids;

src/backend/optimizer/util/pathnode.c

+24-2
Original file line numberDiff line numberDiff line change
@@ -2360,6 +2360,9 @@ create_foreign_upper_path(PlannerInfo *root, RelOptInfo *rel,
23602360
* calc_nestloop_required_outer
23612361
* Compute the required_outer set for a nestloop join path
23622362
*
2363+
* Note: when considering a child join, the inputs nonetheless use top-level
2364+
* parent relids
2365+
*
23632366
* Note: result must not share storage with either input
23642367
*/
23652368
Relids
@@ -2394,11 +2397,30 @@ calc_non_nestloop_required_outer(Path *outer_path, Path *inner_path)
23942397
{
23952398
Relids outer_paramrels = PATH_REQ_OUTER(outer_path);
23962399
Relids inner_paramrels = PATH_REQ_OUTER(inner_path);
2400+
Relids innerrelids PG_USED_FOR_ASSERTS_ONLY;
2401+
Relids outerrelids PG_USED_FOR_ASSERTS_ONLY;
23972402
Relids required_outer;
23982403

2404+
/*
2405+
* Any parameterization of the input paths refers to topmost parents of
2406+
* the relevant relations, because reparameterize_path_by_child() hasn't
2407+
* been called yet. So we must consider topmost parents of the relations
2408+
* being joined, too, while checking for disallowed parameterization
2409+
* cases.
2410+
*/
2411+
if (inner_path->parent->top_parent_relids)
2412+
innerrelids = inner_path->parent->top_parent_relids;
2413+
else
2414+
innerrelids = inner_path->parent->relids;
2415+
2416+
if (outer_path->parent->top_parent_relids)
2417+
outerrelids = outer_path->parent->top_parent_relids;
2418+
else
2419+
outerrelids = outer_path->parent->relids;
2420+
23992421
/* neither path can require rels from the other */
2400-
Assert(!bms_overlap(outer_paramrels, inner_path->parent->relids));
2401-
Assert(!bms_overlap(inner_paramrels, outer_path->parent->relids));
2422+
Assert(!bms_overlap(outer_paramrels, innerrelids));
2423+
Assert(!bms_overlap(inner_paramrels, outerrelids));
24022424
/* form the union ... */
24032425
required_outer = bms_union(outer_paramrels, inner_paramrels);
24042426
/* we do not need an explicit test for empty; bms_union gets it right */

0 commit comments

Comments
 (0)