When removing a relation from the query, drop its RelOptInfo.

In commit b78f6264e I opined that it was "too risky" to delete a
relation's RelOptInfo from the planner's data structures when we have
realized that we don't need to join to it; so instead we just marked
it as a dead relation.  In hindsight that judgment seems flawed: any
subsequent access to such a dead relation is arguably a bug in
itself, so leaving the RelOptInfo present just helps to mask bugs.
Let's delete it instead, allowing removal of the whole notion of a
"dead relation".  So far as the regression tests can find, this
requires no other code changes, except for one Assert in equivclass.c
that was very dubiously not complaining about access to a dead rel.

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

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index de335fdb4d756d62afc31e0f27d7ff5ce02cc523..9132ce235f512031ccb4823d27a865a484df1e73 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -729,8 +729,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
                                continue;
                        }
 
-                       Assert(rel->reloptkind == RELOPT_BASEREL ||
-                                  rel->reloptkind == RELOPT_DEADREL);
+                       Assert(rel->reloptkind == RELOPT_BASEREL);
 
                        rel->eclass_indexes = bms_add_member(rel->eclass_indexes,
                                                                                                 ec_index);

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 7fd11df9afb2297f6dc19b7f67aca9be26eec3ed..0dfefd71f215af3290a7759bf3a203a17f936b28 100644 (file)
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -331,12 +331,6 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
        Index           rti;
        ListCell   *l;
 
-       /*
-        * Mark the rel as "dead" to show it is no longer part of the join tree.
-        * (Removing it from the baserel array altogether seems too risky.)
-        */
-       rel->reloptkind = RELOPT_DEADREL;
-
        /*
         * Remove references to the rel from other baserels' attr_needed arrays.
         */
@@ -487,6 +481,16 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
         * There may be references to the rel in root->fkey_list, but if so,
         * match_foreign_keys_to_quals() will get rid of them.
         */
+
+       /*
+        * Finally, remove the rel from the baserel array to prevent it from being
+        * referenced again.  (We can't do this earlier because
+        * remove_join_clause_from_rels will touch it.)
+        */
+       root->simple_rel_array[relid] = NULL;
+
+       /* And nuke the RelOptInfo, just in case there's another access path */
+       pfree(rel);
 }
 
 /*

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 2b2c6af9ef8019a1da34676265880dfd7fac6a5e..fe4c8c63c95d4a3b3a6867d0ff8a0618869cbc61 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2886,8 +2886,9 @@ match_foreign_keys_to_quals(PlannerInfo *root)
 
                /*
                 * Either relid might identify a rel that is in the query's rtable but
-                * isn't referenced by the jointree so won't have a RelOptInfo.  Hence
-                * don't use find_base_rel() here.  We can ignore such FKs.
+                * isn't referenced by the jointree, or has been removed by join
+                * removal, so that it won't have a RelOptInfo.  Hence don't use
+                * find_base_rel() here.  We can ignore such FKs.
                 */
                if (fkinfo->con_relid >= root->simple_rel_array_size ||
                        fkinfo->ref_relid >= root->simple_rel_array_size)
@@ -2901,8 +2902,7 @@ match_foreign_keys_to_quals(PlannerInfo *root)
 
                /*
                 * Ignore FK unless both rels are baserels.  This gets rid of FKs that
-                * link to inheritance child rels (otherrels) and those that link to
-                * rels removed by join removal (dead rels).
+                * link to inheritance child rels (otherrels).
                 */
                if (con_rel->reloptkind != RELOPT_BASEREL ||
                        ref_rel->reloptkind != RELOPT_BASEREL)

diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index be4d791212c39b72c44c681caea172ad1eea01c5..d61a62da19647d84a6d98c39939500b9523b423c 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -638,9 +638,6 @@ typedef struct PartitionSchemeData *PartitionScheme;
  * Many of the fields in these RelOptInfos are meaningless, but their Path
  * fields always hold Paths showing ways to do that processing step.
  *
- * Lastly, there is a RelOptKind for "dead" relations, which are base rels
- * that we have proven we don't need to join after all.
- *
  * Parts of this data structure are specific to various scan and join
  * mechanisms.  It didn't seem worth creating new node types for them.
  *
@@ -823,8 +820,7 @@ typedef enum RelOptKind
        RELOPT_OTHER_MEMBER_REL,
        RELOPT_OTHER_JOINREL,
        RELOPT_UPPER_REL,
-       RELOPT_OTHER_UPPER_REL,
-       RELOPT_DEADREL
+       RELOPT_OTHER_UPPER_REL
 } RelOptKind;
 
 /*