Fix the issue that SJE mistakenly omits qual clauses

When the SJE code handles the transfer of qual clauses from the removed
relation to the remaining one, it replaces the Vars of the removed
relation with the Vars of the remaining relation for each clause, and
then reintegrates these clauses into the appropriate restriction or join
clause lists, while attempting to avoid duplicates.

However, the code compares RestrictInfo->clause to determine if two
clauses are duplicates.  This is just flat wrong.  Two RestrictInfos
with the same clause can have different required_relids,
incompatible_relids, is_pushed_down, and so on.  This can cause qual
clauses to be mistakenly omitted, leading to wrong results.

This patch fixes it by comparing the entire RestrictInfos not just their
clauses ignoring 'rinfo_serial' field (otherwise almost all RestrictInfos will
be unique).  Making 'rinfo_serial' equal_ignore would break other code.  This
is why this commit implements our own comparison function for checking the
equality of RestrictInfos.

Reported-by: Zuming Jiang
Bug: #18261
Discussion: https://postgr.es/m/18261-2a75d748c928609b%40postgresql.org
Author: Richard Guo

1 files changed, 24 insertions, 2 deletions

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index c053e32e54d..fb01fbe357a 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1658,6 +1658,28 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
 }
 
 /*
+ * "Logically" compares two RestrictInfo's ignoring the 'rinfo_serial' field,
+ * which makes almost every RestrictInfo unique.  This type of comparison is
+ * useful when removing duplicates while moving RestrictInfo's from removed
+ * relation to remaining relation during self-join elimination.
+ *
+ * XXX: In the future, we might remove the 'rinfo_serial' field completely and
+ * get rid of this function.
+ */
+static bool
+restrict_infos_logically_equal(RestrictInfo *a, RestrictInfo *b)
+{
+	int			saved_rinfo_serial = a->rinfo_serial;
+	bool		result;
+
+	a->rinfo_serial = b->rinfo_serial;
+	result = equal(a, b);
+	a->rinfo_serial = saved_rinfo_serial;
+
+	return result;
+}
+
+/*
  * Remove a relation after we have proven that it participates only in an
  * unneeded unique self join.
  *
@@ -1760,7 +1782,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 			if (src == rinfo ||
 				(rinfo->parent_ec != NULL
 				 && src->parent_ec == rinfo->parent_ec)
-				|| equal(rinfo->clause, src->clause))
+				|| restrict_infos_logically_equal(rinfo, src))
 			{
 				is_redundant = true;
 				break;
@@ -1788,7 +1810,7 @@ remove_self_join_rel(PlannerInfo *root, PlanRowMark *kmark, PlanRowMark *rmark,
 			if (src == rinfo ||
 				(rinfo->parent_ec != NULL
 				 && src->parent_ec == rinfo->parent_ec)
-				|| equal(rinfo->clause, src->clause))
+				|| restrict_infos_logically_equal(rinfo, src))
 			{
 				is_redundant = true;
 				break;