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

Commit7481c6c

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 parent13d0053 commit7481c6c

File tree

3 files changed

+94
-42
lines changed

3 files changed

+94
-42
lines changed

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

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,11 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel,
360360
break;
361361
}
362362
}
363+
364+
/*
365+
* We insist that all non-dummy rels have a nonzero rowcount estimate.
366+
*/
367+
Assert(rel->rows>0||IS_DUMMY_REL(rel));
363368
}
364369

365370
/*
@@ -579,6 +584,9 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
579584

580585
/* Let FDW adjust the size estimates, if it can */
581586
rel->fdwroutine->GetForeignRelSize(root,rel,rte->relid);
587+
588+
/* ... but do not let it set the rows estimate to zero */
589+
rel->rows=clamp_row_est(rel->rows);
582590
}
583591

584592
/*
@@ -608,6 +616,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
608616
Indexrti,RangeTblEntry*rte)
609617
{
610618
intparentRTindex=rti;
619+
boolhas_live_children;
611620
doubleparent_rows;
612621
doubleparent_size;
613622
double*parent_attrsizes;
@@ -628,6 +637,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
628637
* Note: if you consider changing this logic, beware that child rels could
629638
* have zero rows and/or width, if they were excluded by constraints.
630639
*/
640+
has_live_children= false;
631641
parent_rows=0;
632642
parent_size=0;
633643
nattrs=rel->max_attr-rel->min_attr+1;
@@ -755,70 +765,80 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
755765
if (IS_DUMMY_REL(childrel))
756766
continue;
757767

768+
/* We have at least one live child. */
769+
has_live_children= true;
770+
758771
/*
759772
* Accumulate size information from each live child.
760773
*/
761-
if (childrel->rows>0)
774+
Assert(childrel->rows>0);
775+
776+
parent_rows+=childrel->rows;
777+
parent_size+=childrel->width*childrel->rows;
778+
779+
/*
780+
* Accumulate per-column estimates too. We need not do anything for
781+
* PlaceHolderVars in the parent list. If child expression isn't a
782+
* Var, or we didn't record a width estimate for it, we have to fall
783+
* back on a datatype-based estimate.
784+
*
785+
* By construction, child's reltargetlist is 1-to-1 with parent's.
786+
*/
787+
forboth(parentvars,rel->reltargetlist,
788+
childvars,childrel->reltargetlist)
762789
{
763-
parent_rows+=childrel->rows;
764-
parent_size+=childrel->width*childrel->rows;
790+
Var*parentvar= (Var*)lfirst(parentvars);
791+
Node*childvar= (Node*)lfirst(childvars);
765792

766-
/*
767-
* Accumulate per-column estimates too. We need not do anything
768-
* for PlaceHolderVars in the parent list. If child expression
769-
* isn't a Var, or we didn't record a width estimate for it, we
770-
* have to fall back on a datatype-based estimate.
771-
*
772-
* By construction, child's reltargetlist is 1-to-1 with parent's.
773-
*/
774-
forboth(parentvars,rel->reltargetlist,
775-
childvars,childrel->reltargetlist)
793+
if (IsA(parentvar,Var))
776794
{
777-
Var*parentvar= (Var*)lfirst(parentvars);
778-
Node*childvar= (Node*)lfirst(childvars);
795+
intpndx=parentvar->varattno-rel->min_attr;
796+
int32child_width=0;
779797

780-
if (IsA(parentvar,Var))
798+
if (IsA(childvar,Var)&&
799+
((Var*)childvar)->varno==childrel->relid)
781800
{
782-
intpndx=parentvar->varattno-rel->min_attr;
783-
int32child_width=0;
801+
intcndx= ((Var*)childvar)->varattno-childrel->min_attr;
784802

785-
if (IsA(childvar,Var)&&
786-
((Var*)childvar)->varno==childrel->relid)
787-
{
788-
intcndx= ((Var*)childvar)->varattno-childrel->min_attr;
789-
790-
child_width=childrel->attr_widths[cndx];
791-
}
792-
if (child_width <=0)
793-
child_width=get_typavgwidth(exprType(childvar),
794-
exprTypmod(childvar));
795-
Assert(child_width>0);
796-
parent_attrsizes[pndx]+=child_width*childrel->rows;
803+
child_width=childrel->attr_widths[cndx];
797804
}
805+
if (child_width <=0)
806+
child_width=get_typavgwidth(exprType(childvar),
807+
exprTypmod(childvar));
808+
Assert(child_width>0);
809+
parent_attrsizes[pndx]+=child_width*childrel->rows;
798810
}
799811
}
800812
}
801813

802-
/*
803-
* Save the finished size estimates.
804-
*/
805-
rel->rows=parent_rows;
806-
if (parent_rows>0)
814+
if (has_live_children)
807815
{
816+
/*
817+
* Save the finished size estimates.
818+
*/
808819
inti;
809820

821+
Assert(parent_rows>0);
822+
rel->rows=parent_rows;
810823
rel->width=rint(parent_size /parent_rows);
811824
for (i=0;i<nattrs;i++)
812825
rel->attr_widths[i]=rint(parent_attrsizes[i] /parent_rows);
826+
827+
/*
828+
* Set "raw tuples" count equal to "rows" for the appendrel; needed
829+
* because some places assume rel->tuples is valid for any baserel.
830+
*/
831+
rel->tuples=parent_rows;
813832
}
814833
else
815-
rel->width=0;/* attr_widths should be zero already */
816-
817-
/*
818-
* Set "raw tuples" count equal to "rows" for the appendrel; needed
819-
* because some places assume rel->tuples is valid for any baserel.
820-
*/
821-
rel->tuples=parent_rows;
834+
{
835+
/*
836+
* All children were excluded by constraints, so mark the whole
837+
* appendrel dummy. We must do this in this phase so that the rel's
838+
* dummy-ness is visible when we generate paths for other rels.
839+
*/
840+
set_dummy_rel_pathlist(rel);
841+
}
822842

823843
pfree(parent_attrsizes);
824844
}

‎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: 12 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
@@ -1120,6 +1131,7 @@ select atts.relid::regclass, s.* from pg_stats s join
11201131
a.attrelid::regclass::textjoin (select unnest(indkey) attnum,
11211132
indexrelidfrom pg_index i) attsonatts.attnum=a.attnumwhere
11221133
schemaname!='pg_catalog';
1134+
11231135
--
11241136
-- Test LATERAL
11251137
--

0 commit comments

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