Propagate lateral-reference information to indirect descendant relations.

create_lateral_join_info() computes a bunch of information about lateral
references between base relations, and then attempts to propagate those
markings to appendrel children of the original base relations.  But the
original coding neglected the possibility of indirect descendants
(grandchildren etc).  During v11 development we noticed that this was
wrong for partitioned-table cases, but failed to realize that it was just
as wrong for any appendrel.  While the case can't arise for appendrels
derived from traditional table inheritance (because we make a flat
appendrel for that), nested appendrels can arise from nested UNION ALL
subqueries.  Failure to mark the lower-level relations as having lateral
references leads to confusion in add_paths_to_append_rel about whether
unparameterized paths can be built.  It's not very clear whether that
leads to any user-visible misbehavior; the lack of field reports suggests
that it may cause nothing worse than minor cost misestimation.  Still,
it's a bug, and it leads to failures of Asserts that I intend to add
later.

To fix, we need to propagate information from all appendrel parents,
not just those that are RELOPT_BASERELs.  We can still do it in one
pass, if we rely on the append_rel_list to be ordered with ancestor
relationships before descendant ones; add assertions checking that.
While fixing this, we can make a small performance improvement by
traversing the append_rel_list just once instead of separately for
each appendrel parent relation.

Noted while investigating bug #15613, though this patch does not fix
that (which is why I'm not committing the related Asserts yet).

Discussion: https://postgr.es/m/3951.1549403812@sss.pgh.pa.us

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index d6ffa7869d1d66478fe88ab939b44f7aaa28bcd1..2afc3f1dfe0194707b585233d938d0ea5eb9fe50 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -419,6 +419,7 @@ void
 create_lateral_join_info(PlannerInfo *root)
 {
        bool            found_laterals = false;
+       Relids          prev_parents PG_USED_FOR_ASSERTS_ONLY = NULL;
        Index           rti;
        ListCell   *lc;
 
@@ -627,53 +628,43 @@ create_lateral_join_info(PlannerInfo *root)
         * every child anyway, and there's no value in forcing extra
         * reparameterize_path() calls.  Similarly, a lateral reference to the
         * parent prevents use of otherwise-movable join rels for each child.
+        *
+        * It's possible for child rels to have their own children, in which case
+        * the topmost parent's lateral info must be propagated all the way down.
+        * This code handles that case correctly so long as append_rel_list has
+        * entries for child relationships before grandchild relationships, which
+        * is an okay assumption right now, but we'll need to be careful to
+        * preserve it.  The assertions below check for incorrect ordering.
         */
-       for (rti = 1; rti < root->simple_rel_array_size; rti++)
+       foreach(lc, root->append_rel_list)
        {
-               RelOptInfo *brel = root->simple_rel_array[rti];
-               RangeTblEntry *brte = root->simple_rte_array[rti];
-
-               /*
-                * Skip empty slots. Also skip non-simple relations i.e. dead
-                * relations.
-                */
-               if (brel == NULL || !IS_SIMPLE_REL(brel))
-                       continue;
+               AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+               RelOptInfo *parentrel = root->simple_rel_array[appinfo->parent_relid];
+               RelOptInfo *childrel = root->simple_rel_array[appinfo->child_relid];
 
                /*
-                * In the case of table inheritance, the parent RTE is directly linked
-                * to every child table via an AppendRelInfo.  In the case of table
-                * partitioning, the inheritance hierarchy is expanded one level at a
-                * time rather than flattened.  Therefore, an other member rel that is
-                * a partitioned table may have children of its own, and must
-                * therefore be marked with the appropriate lateral info so that those
-                * children eventually get marked also.
+                * If we're processing a subquery of a query with inherited target rel
+                * (cf. inheritance_planner), append_rel_list may contain entries for
+                * tables that are not part of the current subquery and hence have no
+                * RelOptInfo.  Ignore them.  We can ignore dead rels, too.
                 */
-               Assert(brte);
-               if (brel->reloptkind == RELOPT_OTHER_MEMBER_REL &&
-                       (brte->rtekind != RTE_RELATION ||
-                        brte->relkind != RELKIND_PARTITIONED_TABLE))
+               if (parentrel == NULL || !IS_SIMPLE_REL(parentrel))
                        continue;
 
-               if (brte->inh)
-               {
-                       foreach(lc, root->append_rel_list)
-                       {
-                               AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
-                               RelOptInfo *childrel;
-
-                               if (appinfo->parent_relid != rti)
-                                       continue;
-                               childrel = root->simple_rel_array[appinfo->child_relid];
-                               Assert(childrel->reloptkind == RELOPT_OTHER_MEMBER_REL);
-                               Assert(childrel->direct_lateral_relids == NULL);
-                               childrel->direct_lateral_relids = brel->direct_lateral_relids;
-                               Assert(childrel->lateral_relids == NULL);
-                               childrel->lateral_relids = brel->lateral_relids;
-                               Assert(childrel->lateral_referencers == NULL);
-                               childrel->lateral_referencers = brel->lateral_referencers;
-                       }
-               }
+               /* Verify that children are processed before grandchildren */
+#ifdef USE_ASSERT_CHECKING
+               prev_parents = bms_add_member(prev_parents, appinfo->parent_relid);
+               Assert(!bms_is_member(appinfo->child_relid, prev_parents));
+#endif
+
+               /* OK, propagate info down */
+               Assert(childrel->reloptkind == RELOPT_OTHER_MEMBER_REL);
+               Assert(childrel->direct_lateral_relids == NULL);
+               childrel->direct_lateral_relids = parentrel->direct_lateral_relids;
+               Assert(childrel->lateral_relids == NULL);
+               childrel->lateral_relids = parentrel->lateral_relids;
+               Assert(childrel->lateral_referencers == NULL);
+               childrel->lateral_referencers = parentrel->lateral_referencers;
        }
 }