Make left-join removal safe under -DREALLOCATE_BITMAPSETS.

tglsfdc · tglsfdc · commit c7be3c015bfe · 2024-05-09T11:01:50.000-04:00
The initial building of RestrictInfos and SpecialJoinInfos tends to create structures that share relid sets (such as syn_lefthand and outer_relids). There's nothing wrong with that in itself, but when we modify those relid sets during join removal, we have to be sure not to corrupt the values that other structures are pointing at. remove_rel_from_query() was careless about this. It accidentally worked anyway because (1) we'd never be reducing the sets to empty, so they wouldn't get pfree'd; and (2) the in-place modification is the same one that we did or will apply to the other struct's relid set, so that there wasn't visible corruption at the end of the process. While there's no live bug in a standard build, of course this is way too fragile to accept going forward. (Maybe we should back-patch this change too for safety, but I've refrained for now.) This bug was exposed by the recent invention of REALLOCATE_BITMAPSETS. Commit e047783 had installed a fix, but that went away with the reversion of SJE, so now we need to fix it again. David Rowley and Tom Lane Discussion: https://postgr.es/m/CACJufxFVQmr4=JWHAOSLuKA5Zy9H26nY6tVrRFBOekHoALyCkQ@mail.gmail.com
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
@@ -390,6 +390,17 @@ remove_rel_from_query(PlannerInfo *root, int relid, SpecialJoinInfo *sjinfo)
 	{
 		SpecialJoinInfo *sjinf = (SpecialJoinInfo *) lfirst(l);
 
+		/*
+		 * initsplan.c is fairly cavalier about allowing SpecialJoinInfos'
+		 * lefthand/righthand relid sets to be shared with other data
+		 * structures.  Ensure that we don't modify the original relid sets.
+		 * (The commute_xxx sets are always per-SpecialJoinInfo though.)
+		 */
+		sjinf->min_lefthand = bms_copy(sjinf->min_lefthand);
+		sjinf->min_righthand = bms_copy(sjinf->min_righthand);
+		sjinf->syn_lefthand = bms_copy(sjinf->syn_lefthand);
+		sjinf->syn_righthand = bms_copy(sjinf->syn_righthand);
+		/* Now remove relid and ojrelid bits from the sets: */
 		sjinf->min_lefthand = bms_del_member(sjinf->min_lefthand, relid);
 		sjinf->min_righthand = bms_del_member(sjinf->min_righthand, relid);
 		sjinf->syn_lefthand = bms_del_member(sjinf->syn_lefthand, relid);
@@ -551,8 +562,11 @@ static void
 remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
 {
 	/*
-	 * The clause_relids probably aren't shared with anything else, but let's
-	 * copy them just to be sure.
+	 * initsplan.c is fairly cavalier about allowing RestrictInfos to share
+	 * relid sets with other RestrictInfos, and SpecialJoinInfos too.  Make
+	 * sure this RestrictInfo has its own relid sets before we modify them.
+	 * (In present usage, clause_relids is probably not shared, but
+	 * required_relids could be; let's not assume anything.)
 	 */
 	rinfo->clause_relids = bms_copy(rinfo->clause_relids);
 	rinfo->clause_relids = bms_del_member(rinfo->clause_relids, relid);
