Outer-join removal does this:
/*
* 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;
which apparently I thought was a good idea in 2010 (cf b78f6264e),
but looking at it now it just seems like an invitation to fail to
detect bugs. We've had a couple of recent reports of indxpath.c
failing like this:
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig(at)entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f0b57bc6859 in __GI_abort () at abort.c:79
#2 0x0000555ec56d3ff3 in ExceptionalCondition (conditionName=0x555ec5887ff0
"outer_rel->rows > 0", fileName=0x555ec5887f2c "indxpath.c",
lineNumber=1909) at assert.c:66
#3 0x0000555ec538a67a in get_loop_count (root=0x555ec5f72680, cur_relid=3,
outer_relids=0x555ec5f93960) at indxpath.c:1909
#4 0x0000555ec5388b5e in build_index_paths (root=0x555ec5f72680,
rel=0x555ec5f8f648, index=0x555ec5f8ca90, clauses=0x7fffeea57480,
useful_predicate=false, scantype=ST_BITMAPSCAN, skip_nonnative_saop=0x0,
skip_lower_saop=0x0) at indxpath.c:957
This is pretty impenetrable at first glance, but what it boils
down to is something accessing a "dead" rel and taking its contents
at face value. Fortunately the contents triggered an assertion,
but that hardly seems like something to count on to detect bugs.
I think it's time to clean this up by removing the rel from the
planner data structures altogether. The attached passes check-world,
and if it does trigger any problems I would say that's a clear
sign of bugs elsewhere.
regards, tom lane
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index de335fdb4d..9132ce235f 100644
--- 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 7fd11df9af..0dfefd71f2 100644
--- 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/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 0d4b1ec4e4..278f7ae80e 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -823,8 +823,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;
/*
