Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8352b14

Browse files
committed
Make entirely-dummy appendrels get marked as such in set_append_rel_size.
The planner generally expects that the estimated rowcount of any relationis at least one row, *unless* it has been proven empty by constraintexclusion or similar mechanisms, which is marked by installing a dummy pathas the rel's cheapest path (cf. IS_DUMMY_REL). When I split upallpaths.c's processing of base rels into separate set_base_rel_sizes andset_base_rel_pathlists steps, the intention was that dummy rels would getmarked as such during the "set size" step; this is what justifies an Assertin indxpath.c's get_loop_count that other relations should either be dummyor have positive rowcount. Unfortunately I didn't get that quite rightfor append relations: if all the child rels have been proven empty thenset_append_rel_size would come up with a rowcount of zero, which iscorrect, but it didn't then do set_dummy_rel_pathlist. (We would haveended up with the right state after set_append_rel_pathlist, but that'stoo late, if we generate indexpaths for some other rel first.)In addition to fixing the actual bug, I installed an Assert enforcing thisconvention in set_rel_size; that then allows simplification of a coupleof now-redundant tests for zero rowcount in set_append_rel_size.Also, to cover the possibility that third-party FDWs have been carelessabout not returning a zero rowcount estimate, apply clamp_row_est towhatever an FDW comes up with as the rows estimate.Per report from Andreas Seltenreich. Back-patch to 9.2. Earlier branchesdid not have the separation between set_base_rel_sizes andset_base_rel_pathlists steps, so there was no intermediate state where anappendrel would have had inconsistent rowcount and pathlist. It's possiblethat adding the Assert to set_rel_size would be a good idea in olderbranches too; but since they're not under development any more, it's likelynot worth the trouble.
1 parent84bf6ec commit8352b14

File tree

3 files changed

+93
-42
lines changed

3 files changed

+93
-42
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,11 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel,
333333
break;
334334
}
335335
}
336+
337+
/*
338+
* We insist that all non-dummy rels have a nonzero rowcount estimate.
339+
*/
340+
Assert(rel->rows>0||IS_DUMMY_REL(rel));
336341
}
337342

338343
/*
@@ -462,6 +467,9 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
462467

463468
/* Let FDW adjust the size estimates, if it can */
464469
rel->fdwroutine->GetForeignRelSize(root,rel,rte->relid);
470+
471+
/* ... but do not let it set the rows estimate to zero */
472+
rel->rows=clamp_row_est(rel->rows);
465473
}
466474

467475
/*
@@ -494,6 +502,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
494502
Indexrti,RangeTblEntry*rte)
495503
{
496504
intparentRTindex=rti;
505+
boolhas_live_children;
497506
doubleparent_rows;
498507
doubleparent_size;
499508
double*parent_attrsizes;
@@ -514,6 +523,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
514523
* Note: if you consider changing this logic, beware that child rels could
515524
* have zero rows and/or width, if they were excluded by constraints.
516525
*/
526+
has_live_children= false;
517527
parent_rows=0;
518528
parent_size=0;
519529
nattrs=rel->max_attr-rel->min_attr+1;
@@ -641,70 +651,80 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
641651
if (IS_DUMMY_REL(childrel))
642652
continue;
643653

654+
/* We have at least one live child. */
655+
has_live_children= true;
656+
644657
/*
645658
* Accumulate size information from each live child.
646659
*/
647-
if (childrel->rows>0)
660+
Assert(childrel->rows>0);
661+
662+
parent_rows+=childrel->rows;
663+
parent_size+=childrel->width*childrel->rows;
664+
665+
/*
666+
* Accumulate per-column estimates too. We need not do anything for
667+
* PlaceHolderVars in the parent list. If child expression isn't a
668+
* Var, or we didn't record a width estimate for it, we have to fall
669+
* back on a datatype-based estimate.
670+
*
671+
* By construction, child's reltargetlist is 1-to-1 with parent's.
672+
*/
673+
forboth(parentvars,rel->reltargetlist,
674+
childvars,childrel->reltargetlist)
648675
{
649-
parent_rows+=childrel->rows;
650-
parent_size+=childrel->width*childrel->rows;
676+
Var*parentvar= (Var*)lfirst(parentvars);
677+
Node*childvar= (Node*)lfirst(childvars);
651678

652-
/*
653-
* Accumulate per-column estimates too. We need not do anything
654-
* for PlaceHolderVars in the parent list. If child expression
655-
* isn't a Var, or we didn't record a width estimate for it, we
656-
* have to fall back on a datatype-based estimate.
657-
*
658-
* By construction, child's reltargetlist is 1-to-1 with parent's.
659-
*/
660-
forboth(parentvars,rel->reltargetlist,
661-
childvars,childrel->reltargetlist)
679+
if (IsA(parentvar,Var))
662680
{
663-
Var*parentvar= (Var*)lfirst(parentvars);
664-
Node*childvar= (Node*)lfirst(childvars);
681+
intpndx=parentvar->varattno-rel->min_attr;
682+
int32child_width=0;
665683

666-
if (IsA(parentvar,Var))
684+
if (IsA(childvar,Var)&&
685+
((Var*)childvar)->varno==childrel->relid)
667686
{
668-
intpndx=parentvar->varattno-rel->min_attr;
669-
int32child_width=0;
687+
intcndx= ((Var*)childvar)->varattno-childrel->min_attr;
670688

671-
if (IsA(childvar,Var)&&
672-
((Var*)childvar)->varno==childrel->relid)
673-
{
674-
intcndx= ((Var*)childvar)->varattno-childrel->min_attr;
675-
676-
child_width=childrel->attr_widths[cndx];
677-
}
678-
if (child_width <=0)
679-
child_width=get_typavgwidth(exprType(childvar),
680-
exprTypmod(childvar));
681-
Assert(child_width>0);
682-
parent_attrsizes[pndx]+=child_width*childrel->rows;
689+
child_width=childrel->attr_widths[cndx];
683690
}
691+
if (child_width <=0)
692+
child_width=get_typavgwidth(exprType(childvar),
693+
exprTypmod(childvar));
694+
Assert(child_width>0);
695+
parent_attrsizes[pndx]+=child_width*childrel->rows;
684696
}
685697
}
686698
}
687699

688-
/*
689-
* Save the finished size estimates.
690-
*/
691-
rel->rows=parent_rows;
692-
if (parent_rows>0)
700+
if (has_live_children)
693701
{
702+
/*
703+
* Save the finished size estimates.
704+
*/
694705
inti;
695706

707+
Assert(parent_rows>0);
708+
rel->rows=parent_rows;
696709
rel->width=rint(parent_size /parent_rows);
697710
for (i=0;i<nattrs;i++)
698711
rel->attr_widths[i]=rint(parent_attrsizes[i] /parent_rows);
712+
713+
/*
714+
* Set "raw tuples" count equal to "rows" for the appendrel; needed
715+
* because some places assume rel->tuples is valid for any baserel.
716+
*/
717+
rel->tuples=parent_rows;
699718
}
700719
else
701-
rel->width=0;/* attr_widths should be zero already */
702-
703-
/*
704-
* Set "raw tuples" count equal to "rows" for the appendrel; needed
705-
* because some places assume rel->tuples is valid for any baserel.
706-
*/
707-
rel->tuples=parent_rows;
720+
{
721+
/*
722+
* All children were excluded by constraints, so mark the whole
723+
* appendrel dummy. We must do this in this phase so that the rel's
724+
* dummy-ness is visible when we generate paths for other rels.
725+
*/
726+
set_dummy_rel_pathlist(rel);
727+
}
708728

709729
pfree(parent_attrsizes);
710730
}

‎src/test/regress/expected/join.out

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,6 +2164,26 @@ select count(*) from tenk1 x where
21642164
(1 row)
21652165

21662166
rollback;
2167+
--
2168+
-- regression test: be sure we cope with proven-dummy append rels
2169+
--
2170+
explain (costs off)
2171+
select aa, bb, unique1, unique1
2172+
from tenk1 right join b on aa = unique1
2173+
where bb < bb and bb is null;
2174+
QUERY PLAN
2175+
--------------------------
2176+
Result
2177+
One-Time Filter: false
2178+
(2 rows)
2179+
2180+
select aa, bb, unique1, unique1
2181+
from tenk1 right join b on aa = unique1
2182+
where bb < bb and bb is null;
2183+
aa | bb | unique1 | unique1
2184+
----+----+---------+---------
2185+
(0 rows)
2186+
21672187
--
21682188
-- Clean up
21692189
--

‎src/test/regress/sql/join.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,17 @@ select count(*) from tenk1 x where
353353
x.unique1in (selectaa.f1from int4_tbl aa,float8_tbl bbwhereaa.f1=bb.f1);
354354
rollback;
355355

356+
--
357+
-- regression test: be sure we cope with proven-dummy append rels
358+
--
359+
explain (costs off)
360+
select aa, bb, unique1, unique1
361+
from tenk1right join bon aa= unique1
362+
where bb< bband bb isnull;
363+
364+
select aa, bb, unique1, unique1
365+
from tenk1right join bon aa= unique1
366+
where bb< bband bb isnull;
356367

357368
--
358369
-- Clean up

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp