Re: BUG #19460: FULL JOIN rewriting issue on empty queries - Mailing list pgsql-bugs

PG Bug reporting form <noreply@postgresql.org> writes:
> After migrating from version 15 to 17.9, the following query fails with:
> ERROR: FULL JOIN is only supported with merge-joinable or hash-joinable join
> conditions

Thanks for the report!

This turns out to be because somebody long ago thought that outer join
removal could be lazy about how much of the planner's data structures
it needs to update.  Specifically, when the lower LEFT OUTER JOIN
gets removed, we failed to remove the associated relids from the
left_relids and right_relids of the upper "ON rhs.id = lhs.id" clause,
and that blocks recognition of the applicability of a hash or merge
join, because clause_sides_match_join() fails.

The fix seems pretty trivial, as attached.  (While I'm only certain
that we have to fix left_relids and right_relids, this discovery
makes it seem like it'd be pretty foolish not to fix all the relid
sets of a RestrictInfo.)  I didn't make a regression test case yet,
but we need one since no existing test results change (!?).

I'm feeling a tad nervous about pushing this into released branches.
It seems likely that it might enable quite a few join plans that were
previously not considered, and people tend not to like plan changes in
stable branches.  However, (a) it's hard to argue that this isn't a
regression from pre-v16, and (b) since this change affects no existing
test, maybe the blast radius isn't as big as I fear.

Thoughts?

            regards, tom lane

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 12e9ed0d0c7..72f0d018f3d 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -650,7 +650,7 @@ remove_leftjoinrel_from_query(PlannerInfo *root, int relid,
 /*
  * Remove any references to relid or ojrelid from the RestrictInfo.
  *
- * We only bother to clean out bits in clause_relids and required_relids,
+ * We only bother to clean out bits in the RestrictInfo's various relid sets,
  * not nullingrel bits in contained Vars and PHVs.  (This might have to be
  * improved sometime.)  However, if the RestrictInfo contains an OR clause
  * we have to also clean up the sub-clauses.
@@ -672,6 +672,22 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
     rinfo->required_relids = bms_copy(rinfo->required_relids);
     rinfo->required_relids = bms_del_member(rinfo->required_relids, relid);
     rinfo->required_relids = bms_del_member(rinfo->required_relids, ojrelid);
+    /* Likewise for incompatible_relids */
+    rinfo->incompatible_relids = bms_copy(rinfo->incompatible_relids);
+    rinfo->incompatible_relids = bms_del_member(rinfo->incompatible_relids, relid);
+    rinfo->incompatible_relids = bms_del_member(rinfo->incompatible_relids, ojrelid);
+    /* Likewise for outer_relids */
+    rinfo->outer_relids = bms_copy(rinfo->outer_relids);
+    rinfo->outer_relids = bms_del_member(rinfo->outer_relids, relid);
+    rinfo->outer_relids = bms_del_member(rinfo->outer_relids, ojrelid);
+    /* Likewise for left_relids */
+    rinfo->left_relids = bms_copy(rinfo->left_relids);
+    rinfo->left_relids = bms_del_member(rinfo->left_relids, relid);
+    rinfo->left_relids = bms_del_member(rinfo->left_relids, ojrelid);
+    /* Likewise for right_relids */
+    rinfo->right_relids = bms_copy(rinfo->right_relids);
+    rinfo->right_relids = bms_del_member(rinfo->right_relids, relid);
+    rinfo->right_relids = bms_del_member(rinfo->right_relids, ojrelid);

     /* If it's an OR, recurse to clean up sub-clauses */
     if (restriction_is_or_clause(rinfo))