database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-09-09 13:09:39 +03:00
Code Activity

Fix the issue that SJE mistakenly omits qual clauses

Browse Source 
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
This commit is contained in:
Alexander Korotkov
2024-01-06 14:09:39 +02:00
parent 43b46aae12
commit 5ef34a8fc3
3 changed files with 65 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
src/backend/optimizer/plan/analyzejoins.c
View File

@@ -1657,6 +1657,28 @@ update_eclasses(EquivalenceClass *ec, int from, int to)
ec->ec_relids = replace_relid(ec->ec_relids, from, 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;