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.

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 1590be116750846b8957fe8a9ae1ed03b89d6917..8fc1cfd15f5330a44c537eec49e5eecc93dac27f 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -360,6 +360,11 @@ set_rel_size(PlannerInfo *root, RelOptInfo *rel,
                                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)
 
        /* Let FDW adjust the size estimates, if it can */
        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,
                                        Index rti, RangeTblEntry *rte)
 {
        int                     parentRTindex = rti;
+       bool            has_live_children;
        double          parent_rows;
        double          parent_size;
        double     *parent_attrsizes;
@@ -628,6 +637,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
         * Note: if you consider changing this logic, beware that child rels could
         * have zero rows and/or width, if they were excluded by constraints.
         */
+       has_live_children = false;
        parent_rows = 0;
        parent_size = 0;
        nattrs = rel->max_attr - rel->min_attr + 1;
@@ -755,70 +765,80 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
                if (IS_DUMMY_REL(childrel))
                        continue;
 
+               /* We have at least one live child. */
+               has_live_children = true;
+
                /*
                 * 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;
-                       parent_size += childrel->width * childrel->rows;
+                       Var                *parentvar = (Var *) lfirst(parentvars);
+                       Node       *childvar = (Node *) lfirst(childvars);
 
-                       /*
-                        * 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)
+                       if (IsA(parentvar, Var))
                        {
-                               Var                *parentvar = (Var *) lfirst(parentvars);
-                               Node       *childvar = (Node *) lfirst(childvars);
+                               int                     pndx = parentvar->varattno - rel->min_attr;
+                               int32           child_width = 0;
 
-                               if (IsA(parentvar, Var))
+                               if (IsA(childvar, Var) &&
+                                       ((Var *) childvar)->varno == childrel->relid)
                                {
-                                       int                     pndx = parentvar->varattno - rel->min_attr;
-                                       int32           child_width = 0;
+                                       int                     cndx = ((Var *) childvar)->varattno - childrel->min_attr;
 
-                                       if (IsA(childvar, Var) &&
-                                               ((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;
+                                       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;
                        }
                }
        }
 
-       /*
-        * Save the finished size estimates.
-        */
-       rel->rows = parent_rows;
-       if (parent_rows > 0)
+       if (has_live_children)
        {
+               /*
+                * Save the finished size estimates.
+                */
                int                     i;
 
+               Assert(parent_rows > 0);
+               rel->rows = parent_rows;
                rel->width = rint(parent_size / parent_rows);
                for (i = 0; i < nattrs; i++)
                        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
-               rel->width = 0;                 /* attr_widths should be zero already */
-
-       /*
-        * 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;
+       {
+               /*
+                * All children were excluded by constraints, so mark the whole
+                * appendrel dummy.  We must do this in this phase so that the rel's
+                * dummy-ness is visible when we generate paths for other rels.
+                */
+               set_dummy_rel_pathlist(rel);
+       }
 
        pfree(parent_attrsizes);
 }

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 139f7e049874bd08ce76a9468e0acd6d2f51e3c6..96ec997ed1661005a5a470ffd19eed3653cdb2ec 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2164,6 +2164,26 @@ select count(*) from tenk1 x where
 (1 row)
 
 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
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 5b65ea8c9224ace3b18b10116f532ca1dc01c41d..ada78db2644707cd94e79fab68a1696708d522d2 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -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);
 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
@@ -1120,6 +1131,7 @@ select atts.relid::regclass, s.* from pg_stats s join
     a.attrelid::regclass::text join (select unnest(indkey) attnum,
     indexrelid from pg_index i) atts on atts.attnum = a.attnum where
     schemaname != 'pg_catalog';
+
 --
 -- Test LATERAL
 --