Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e9a20e4

Browse files
committed
When removing a relation from the query, drop its RelOptInfo.
In commit b78f626 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
1 parent c7468c7 commit e9a20e4

File tree

4 files changed

+16
-17
lines changed

4 files changed

+16
-17
lines changed

src/backend/optimizer/path/equivclass.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,8 +729,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
729729
continue;
730730
}
731731

732-
Assert(rel->reloptkind == RELOPT_BASEREL ||
733-
rel->reloptkind == RELOPT_DEADREL);
732+
Assert(rel->reloptkind == RELOPT_BASEREL);
734733

735734
rel->eclass_indexes = bms_add_member(rel->eclass_indexes,
736735
ec_index);

src/backend/optimizer/plan/analyzejoins.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,12 +331,6 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
331331
Index rti;
332332
ListCell *l;
333333

334-
/*
335-
* Mark the rel as "dead" to show it is no longer part of the join tree.
336-
* (Removing it from the baserel array altogether seems too risky.)
337-
*/
338-
rel->reloptkind = RELOPT_DEADREL;
339-
340334
/*
341335
* Remove references to the rel from other baserels' attr_needed arrays.
342336
*/
@@ -487,6 +481,16 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
487481
* There may be references to the rel in root->fkey_list, but if so,
488482
* match_foreign_keys_to_quals() will get rid of them.
489483
*/
484+
485+
/*
486+
* Finally, remove the rel from the baserel array to prevent it from being
487+
* referenced again. (We can't do this earlier because
488+
* remove_join_clause_from_rels will touch it.)
489+
*/
490+
root->simple_rel_array[relid] = NULL;
491+
492+
/* And nuke the RelOptInfo, just in case there's another access path */
493+
pfree(rel);
490494
}
491495

492496
/*

src/backend/optimizer/plan/initsplan.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2886,8 +2886,9 @@ match_foreign_keys_to_quals(PlannerInfo *root)
28862886

28872887
/*
28882888
* Either relid might identify a rel that is in the query's rtable but
2889-
* isn't referenced by the jointree so won't have a RelOptInfo. Hence
2890-
* don't use find_base_rel() here. We can ignore such FKs.
2889+
* isn't referenced by the jointree, or has been removed by join
2890+
* removal, so that it won't have a RelOptInfo. Hence don't use
2891+
* find_base_rel() here. We can ignore such FKs.
28912892
*/
28922893
if (fkinfo->con_relid >= root->simple_rel_array_size ||
28932894
fkinfo->ref_relid >= root->simple_rel_array_size)
@@ -2901,8 +2902,7 @@ match_foreign_keys_to_quals(PlannerInfo *root)
29012902

29022903
/*
29032904
* Ignore FK unless both rels are baserels. This gets rid of FKs that
2904-
* link to inheritance child rels (otherrels) and those that link to
2905-
* rels removed by join removal (dead rels).
2905+
* link to inheritance child rels (otherrels).
29062906
*/
29072907
if (con_rel->reloptkind != RELOPT_BASEREL ||
29082908
ref_rel->reloptkind != RELOPT_BASEREL)

src/include/nodes/pathnodes.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,6 @@ typedef struct PartitionSchemeData *PartitionScheme;
638638
* Many of the fields in these RelOptInfos are meaningless, but their Path
639639
* fields always hold Paths showing ways to do that processing step.
640640
*
641-
* Lastly, there is a RelOptKind for "dead" relations, which are base rels
642-
* that we have proven we don't need to join after all.
643-
*
644641
* Parts of this data structure are specific to various scan and join
645642
* mechanisms. It didn't seem worth creating new node types for them.
646643
*
@@ -823,8 +820,7 @@ typedef enum RelOptKind
823820
RELOPT_OTHER_MEMBER_REL,
824821
RELOPT_OTHER_JOINREL,
825822
RELOPT_UPPER_REL,
826-
RELOPT_OTHER_UPPER_REL,
827-
RELOPT_DEADREL
823+
RELOPT_OTHER_UPPER_REL
828824
} RelOptKind;
829825

830826
/*

0 commit comments

Comments
 (0)