Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 44e56ba

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix join removal logic to clean up sub-RestrictInfos of OR clauses.
analyzejoins.c took care to clean out removed relids from the clause_relids and required_relids of RestrictInfos associated with the doomed rel ... but it paid no attention to the fact that if such a RestrictInfo contains an OR clause, there will be sub-RestrictInfos containing similar fields. I'm more than a bit surprised that this oversight hasn't caused visible problems before. In any case, it's certainly broken now, so add logic to clean out the sub-RestrictInfos recursively. We might need to back-patch this someday. Per bug #17786 from Robins Tharakan. Discussion: https://postgr.es/m/17786-f1ea7fbdab97daec@postgresql.org
1 parent acc5821 commit 44e56ba
Copy full SHA for 44e56ba

File tree

3 files changed

+90
-15
lines changed
Filter options

3 files changed

+90
-15
lines changed

‎src/backend/optimizer/plan/analyzejoins.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/plan/analyzejoins.c
+62-15Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,16 @@
2929
#include "optimizer/pathnode.h"
3030
#include "optimizer/paths.h"
3131
#include "optimizer/planmain.h"
32+
#include "optimizer/restrictinfo.h"
3233
#include "optimizer/tlist.h"
3334
#include "utils/lsyscache.h"
3435

3536
/* local functions */
3637
static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo);
3738
static void remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
3839
Relids joinrelids);
40+
static void remove_rel_from_restrictinfo(RestrictInfo *rinfo,
41+
int relid, int ojrelid);
3942
static List *remove_rel_from_joinlist(List *joinlist, int relid, int *nremoved);
4043
static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel);
4144
static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel,
@@ -470,24 +473,12 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
470473
{
471474
/*
472475
* There might be references to relid or ojrelid in the
473-
* clause_relids as a consequence of PHVs having ph_eval_at sets
476+
* RestrictInfo, as a consequence of PHVs having ph_eval_at sets
474477
* that include those. We already checked above that any such PHV
475478
* is safe, so we can just drop those references.
476-
*
477-
* The clause_relids probably aren't shared with anything else,
478-
* but let's copy them just to be sure.
479479
*/
480-
rinfo->clause_relids = bms_copy(rinfo->clause_relids);
481-
rinfo->clause_relids = bms_del_member(rinfo->clause_relids,
482-
relid);
483-
rinfo->clause_relids = bms_del_member(rinfo->clause_relids,
484-
ojrelid);
485-
/* Likewise for required_relids */
486-
rinfo->required_relids = bms_copy(rinfo->required_relids);
487-
rinfo->required_relids = bms_del_member(rinfo->required_relids,
488-
relid);
489-
rinfo->required_relids = bms_del_member(rinfo->required_relids,
490-
ojrelid);
480+
remove_rel_from_restrictinfo(rinfo, relid, ojrelid);
481+
/* Now throw it back into the joininfo lists */
491482
distribute_restrictinfo_to_rels(root, rinfo);
492483
}
493484
}
@@ -498,6 +489,62 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
498489
*/
499490
}
500491

492+
/*
493+
* Remove any references to relid or ojrelid from the RestrictInfo.
494+
*
495+
* We only bother to clean out bits in clause_relids and required_relids,
496+
* not nullingrel bits in contained Vars and PHVs. (This might have to be
497+
* improved sometime.) However, if the RestrictInfo contains an OR clause
498+
* we have to also clean up the sub-clauses.
499+
*/
500+
static void
501+
remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
502+
{
503+
/*
504+
* The clause_relids probably aren't shared with anything else, but let's
505+
* copy them just to be sure.
506+
*/
507+
rinfo->clause_relids = bms_copy(rinfo->clause_relids);
508+
rinfo->clause_relids = bms_del_member(rinfo->clause_relids, relid);
509+
rinfo->clause_relids = bms_del_member(rinfo->clause_relids, ojrelid);
510+
/* Likewise for required_relids */
511+
rinfo->required_relids = bms_copy(rinfo->required_relids);
512+
rinfo->required_relids = bms_del_member(rinfo->required_relids, relid);
513+
rinfo->required_relids = bms_del_member(rinfo->required_relids, ojrelid);
514+
515+
/* If it's an OR, recurse to clean up sub-clauses */
516+
if (restriction_is_or_clause(rinfo))
517+
{
518+
ListCell *lc;
519+
520+
Assert(is_orclause(rinfo->orclause));
521+
foreach(lc, ((BoolExpr *) rinfo->orclause)->args)
522+
{
523+
Node *orarg = (Node *) lfirst(lc);
524+
525+
/* OR arguments should be ANDs or sub-RestrictInfos */
526+
if (is_andclause(orarg))
527+
{
528+
List *andargs = ((BoolExpr *) orarg)->args;
529+
ListCell *lc2;
530+
531+
foreach(lc2, andargs)
532+
{
533+
RestrictInfo *rinfo2 = lfirst_node(RestrictInfo, lc2);
534+
535+
remove_rel_from_restrictinfo(rinfo2, relid, ojrelid);
536+
}
537+
}
538+
else
539+
{
540+
RestrictInfo *rinfo2 = castNode(RestrictInfo, orarg);
541+
542+
remove_rel_from_restrictinfo(rinfo2, relid, ojrelid);
543+
}
544+
}
545+
}
546+
}
547+
501548
/*
502549
* Remove any occurrences of the target relid from a joinlist structure.
503550
*

‎src/test/regress/expected/join.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/join.out
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5344,6 +5344,26 @@ SELECT q2 FROM
53445344
One-Time Filter: false
53455345
(9 rows)
53465346

5347+
-- join removal bug #17786: check that OR conditions are cleaned up
5348+
EXPLAIN (COSTS OFF)
5349+
SELECT f1, x
5350+
FROM int4_tbl
5351+
JOIN ((SELECT 42 AS x FROM int8_tbl LEFT JOIN innertab ON q1 = id) AS ss1
5352+
RIGHT JOIN tenk1 ON NULL)
5353+
ON tenk1.unique1 = ss1.x OR tenk1.unique2 = ss1.x;
5354+
QUERY PLAN
5355+
--------------------------------------------------------------------------
5356+
Nested Loop
5357+
-> Seq Scan on int4_tbl
5358+
-> Materialize
5359+
-> Nested Loop Left Join
5360+
Join Filter: NULL::boolean
5361+
Filter: ((tenk1.unique1 = (42)) OR (tenk1.unique2 = (42)))
5362+
-> Seq Scan on tenk1
5363+
-> Result
5364+
One-Time Filter: false
5365+
(9 rows)
5366+
53475367
rollback;
53485368
-- another join removal bug: we must clean up correctly when removing a PHV
53495369
begin;

‎src/test/regress/sql/join.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/join.sql
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,6 +1955,14 @@ SELECT q2 FROM
19551955
RIGHT JOIN int4_tbl ON NULL
19561956
WHERE x >= x;
19571957

1958+
-- join removal bug #17786: check that OR conditions are cleaned up
1959+
EXPLAIN (COSTS OFF)
1960+
SELECT f1, x
1961+
FROM int4_tbl
1962+
JOIN ((SELECT 42 AS x FROM int8_tbl LEFT JOIN innertab ON q1 = id) AS ss1
1963+
RIGHT JOIN tenk1 ON NULL)
1964+
ON tenk1.unique1 = ss1.x OR tenk1.unique2 = ss1.x;
1965+
19581966
rollback;
19591967

19601968
-- another join removal bug: we must clean up correctly when removing a PHV

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.