Commit 358eaa01 authored by Tom Lane's avatar Tom Lane

Make entirely-dummy appendrels get marked as such in set_append_rel_size.

The planner generally expects that the estimated rowcount of any relation
is at least one row, *unless* it has been proven empty by constraint
exclusion or similar mechanisms, which is marked by installing a dummy path
as the rel's cheapest path (cf. IS_DUMMY_REL).  When I split up
allpaths.c's processing of base rels into separate set_base_rel_sizes and
set_base_rel_pathlists steps, the intention was that dummy rels would get
marked as such during the "set size" step; this is what justifies an Assert
in indxpath.c's get_loop_count that other relations should either be dummy
or have positive rowcount.  Unfortunately I didn't get that quite right
for append relations: if all the child rels have been proven empty then
set_append_rel_size would come up with a rowcount of zero, which is
correct, but it didn't then do set_dummy_rel_pathlist.  (We would have
ended up with the right state after set_append_rel_pathlist, but that's
too late, if we generate indexpaths for some other rel first.)

In addition to fixing the actual bug, I installed an Assert enforcing this
convention in set_rel_size; that then allows simplification of a couple
of now-redundant tests for zero rowcount in set_append_rel_size.

Also, to cover the possibility that third-party FDWs have been careless
about not returning a zero rowcount estimate, apply clamp_row_est to
whatever an FDW comes up with as the rows estimate.

Per report from Andreas Seltenreich.  Back-patch to 9.2.  Earlier branches
did not have the separation between set_base_rel_sizes and
set_base_rel_pathlists steps, so there was no intermediate state where an
appendrel would have had inconsistent rowcount and pathlist.  It's possible
that adding the Assert to set_rel_size would be a good idea in older
branches too; but since they're not under development any more, it's likely
not worth the trouble.
parent 159cff58
...@@ -360,6 +360,11 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel, ...@@ -360,6 +360,11 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel,
break; break;
} }
} }
/*
* We insist that all non-dummy rels have a nonzero rowcount estimate.
*/
Assert(rel->rows > 0 || IS_DUMMY_REL(rel));
} }
/* /*
...@@ -579,6 +584,9 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte) ...@@ -579,6 +584,9 @@ set_foreign_size(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
/* Let FDW adjust the size estimates, if it can */ /* Let FDW adjust the size estimates, if it can */
rel->fdwroutine->GetForeignRelSize(root, rel, rte->relid); rel->fdwroutine->GetForeignRelSize(root, rel, rte->relid);
/* ... but do not let it set the rows estimate to zero */
rel->rows = clamp_row_est(rel->rows);
} }
/* /*
...@@ -608,6 +616,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, ...@@ -608,6 +616,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
Index rti, RangeTblEntry *rte) Index rti, RangeTblEntry *rte)
{ {
int parentRTindex = rti; int parentRTindex = rti;
bool has_live_children;
double parent_rows; double parent_rows;
double parent_size; double parent_size;
double *parent_attrsizes; double *parent_attrsizes;
...@@ -628,6 +637,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, ...@@ -628,6 +637,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
* Note: if you consider changing this logic, beware that child rels could * Note: if you consider changing this logic, beware that child rels could
* have zero rows and/or width, if they were excluded by constraints. * have zero rows and/or width, if they were excluded by constraints.
*/ */
has_live_children = false;
parent_rows = 0; parent_rows = 0;
parent_size = 0; parent_size = 0;
nattrs = rel->max_attr - rel->min_attr + 1; nattrs = rel->max_attr - rel->min_attr + 1;
...@@ -755,70 +765,80 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, ...@@ -755,70 +765,80 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
if (IS_DUMMY_REL(childrel)) if (IS_DUMMY_REL(childrel))
continue; continue;
/* We have at least one live child. */
has_live_children = true;
/* /*
* Accumulate size information from each live child. * Accumulate size information from each live child.
*/ */
if (childrel->rows > 0) Assert(childrel->rows > 0);
parent_rows += childrel->rows;
parent_size += childrel->width * childrel->rows;
/*
* Accumulate per-column estimates too. We need not do anything for
* PlaceHolderVars in the parent list. If child expression isn't a
* Var, or we didn't record a width estimate for it, we have to fall
* back on a datatype-based estimate.
*
* By construction, child's reltargetlist is 1-to-1 with parent's.
*/
forboth(parentvars, rel->reltargetlist,
childvars, childrel->reltargetlist)
{ {
parent_rows += childrel->rows; Var *parentvar = (Var *) lfirst(parentvars);
parent_size += childrel->width * childrel->rows; Node *childvar = (Node *) lfirst(childvars);
/* if (IsA(parentvar, Var))
* Accumulate per-column estimates too. We need not do anything
* for PlaceHolderVars in the parent list. If child expression
* isn't a Var, or we didn't record a width estimate for it, we
* have to fall back on a datatype-based estimate.
*
* By construction, child's reltargetlist is 1-to-1 with parent's.
*/
forboth(parentvars, rel->reltargetlist,
childvars, childrel->reltargetlist)
{ {
Var *parentvar = (Var *) lfirst(parentvars); int pndx = parentvar->varattno - rel->min_attr;
Node *childvar = (Node *) lfirst(childvars); int32 child_width = 0;
if (IsA(parentvar, Var)) if (IsA(childvar, Var) &&
((Var *) childvar)->varno == childrel->relid)
{ {
int pndx = parentvar->varattno - rel->min_attr; int cndx = ((Var *) childvar)->varattno - childrel->min_attr;
int32 child_width = 0;
if (IsA(childvar, Var) && child_width = childrel->attr_widths[cndx];
((Var *) childvar)->varno == childrel->relid)
{
int cndx = ((Var *) childvar)->varattno - childrel->min_attr;
child_width = childrel->attr_widths[cndx];
}
if (child_width <= 0)
child_width = get_typavgwidth(exprType(childvar),
exprTypmod(childvar));
Assert(child_width > 0);
parent_attrsizes[pndx] += child_width * childrel->rows;
} }
if (child_width <= 0)
child_width = get_typavgwidth(exprType(childvar),
exprTypmod(childvar));
Assert(child_width > 0);
parent_attrsizes[pndx] += child_width * childrel->rows;
} }
} }
} }
/* if (has_live_children)
* Save the finished size estimates.
*/
rel->rows = parent_rows;
if (parent_rows > 0)
{ {
/*
* Save the finished size estimates.
*/
int i; int i;
Assert(parent_rows > 0);
rel->rows = parent_rows;
rel->width = rint(parent_size / parent_rows); rel->width = rint(parent_size / parent_rows);
for (i = 0; i < nattrs; i++) for (i = 0; i < nattrs; i++)
rel->attr_widths[i] = rint(parent_attrsizes[i] / parent_rows); rel->attr_widths[i] = rint(parent_attrsizes[i] / parent_rows);
/*
* Set "raw tuples" count equal to "rows" for the appendrel; needed
* because some places assume rel->tuples is valid for any baserel.
*/
rel->tuples = parent_rows;
} }
else else
rel->width = 0; /* attr_widths should be zero already */ {
/*
/* * All children were excluded by constraints, so mark the whole
* Set "raw tuples" count equal to "rows" for the appendrel; needed * appendrel dummy. We must do this in this phase so that the rel's
* because some places assume rel->tuples is valid for any baserel. * dummy-ness is visible when we generate paths for other rels.
*/ */
rel->tuples = parent_rows; set_dummy_rel_pathlist(rel);
}
pfree(parent_attrsizes); pfree(parent_attrsizes);
} }
......
...@@ -2164,6 +2164,26 @@ select count(*) from tenk1 x where ...@@ -2164,6 +2164,26 @@ select count(*) from tenk1 x where
(1 row) (1 row)
rollback; rollback;
--
-- regression test: be sure we cope with proven-dummy append rels
--
explain (costs off)
select aa, bb, unique1, unique1
from tenk1 right join b on aa = unique1
where bb < bb and bb is null;
QUERY PLAN
--------------------------
Result
One-Time Filter: false
(2 rows)
select aa, bb, unique1, unique1
from tenk1 right join b on aa = unique1
where bb < bb and bb is null;
aa | bb | unique1 | unique1
----+----+---------+---------
(0 rows)
-- --
-- Clean up -- Clean up
-- --
......
...@@ -353,6 +353,17 @@ select count(*) from tenk1 x where ...@@ -353,6 +353,17 @@ select count(*) from tenk1 x where
x.unique1 in (select aa.f1 from int4_tbl aa,float8_tbl bb where aa.f1=bb.f1); x.unique1 in (select aa.f1 from int4_tbl aa,float8_tbl bb where aa.f1=bb.f1);
rollback; rollback;
--
-- regression test: be sure we cope with proven-dummy append rels
--
explain (costs off)
select aa, bb, unique1, unique1
from tenk1 right join b on aa = unique1
where bb < bb and bb is null;
select aa, bb, unique1, unique1
from tenk1 right join b on aa = unique1
where bb < bb and bb is null;
-- --
-- Clean up -- Clean up
...@@ -1120,6 +1131,7 @@ select atts.relid::regclass, s.* from pg_stats s join ...@@ -1120,6 +1131,7 @@ select atts.relid::regclass, s.* from pg_stats s join
a.attrelid::regclass::text join (select unnest(indkey) attnum, a.attrelid::regclass::text join (select unnest(indkey) attnum,
indexrelid from pg_index i) atts on atts.attnum = a.attnum where indexrelid from pg_index i) atts on atts.attnum = a.attnum where
schemaname != 'pg_catalog'; schemaname != 'pg_catalog';
-- --
-- Test LATERAL -- Test LATERAL
-- --
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment