pg_plan_advice: Fix another unique-semijoin bug.

This one occurs when an outer join appears beneath the made-unique
side of a semijoin. The issue is that join RTEs are not featured
out of sj_unique_rels entries. Fix, and add a test case.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Analyzed-by: Tender Wang <tndrwang@gmail.com>
Discussion: http://postgr.es/m/c0c63979-43c2-4424-8fe8-56949934c9d8@gmail.com

diff --git a/contrib/pg_plan_advice/expected/semijoin.out b/contrib/pg_plan_advice/expected/semijoin.out
index 680de215117e616ca22848ac5a51e35b28cc0de8..db6b069ec8eeb8c1519e30566f510b49a5f41554 100644 (file)
--- a/contrib/pg_plan_advice/expected/semijoin.out
+++ b/contrib/pg_plan_advice/expected/semijoin.out
@@ -392,3 +392,35 @@ SELECT * FROM
    NO_GATHER(x)
 (5 rows)
 
+-- Test the case where the planner makes one side of a semijoin unique, and
+-- that side contains an outer join; this test is just to make sure that
+-- advice generation does not fail.
+EXPLAIN (COSTS OFF, PLAN_ADVICE)
+SELECT 1 FROM generate_series(1, 1000) g WHERE EXISTS
+       (SELECT 1 FROM
+               (SELECT 1 FROM (SELECT 1) LEFT JOIN sj_narrow ON true) s,
+               sj_narrow t2 WHERE g = t2.id);
+                               QUERY PLAN                               
+------------------------------------------------------------------------
+ Hash Join
+   Hash Cond: (t2.id = g.g)
+   ->  Unique
+         ->  Nested Loop
+               ->  Index Only Scan using sj_narrow_pkey on sj_narrow t2
+               ->  Materialize
+                     ->  Nested Loop Left Join
+                           ->  Result
+                           ->  Seq Scan on sj_narrow
+   ->  Hash
+         ->  Function Scan on generate_series g
+ Generated Plan Advice:
+   JOIN_ORDER(t2 ("*RESULT*" sj_narrow) g)
+   NESTED_LOOP_PLAIN(sj_narrow)
+   NESTED_LOOP_MATERIALIZE((sj_narrow "*RESULT*"))
+   HASH_JOIN(g)
+   SEQ_SCAN(sj_narrow)
+   INDEX_ONLY_SCAN(t2 public.sj_narrow_pkey)
+   SEMIJOIN_UNIQUE((t2 sj_narrow "*RESULT*"))
+   NO_GATHER(g t2 sj_narrow "*RESULT*")
+(20 rows)
+

diff --git a/contrib/pg_plan_advice/pgpa_planner.c b/contrib/pg_plan_advice/pgpa_planner.c
index 72ef3230abc418550395e068af357bbef7c7e739..b2662bd410f3292b101442461880dc471a88d56d 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_planner.c
+++ b/contrib/pg_plan_advice/pgpa_planner.c
@@ -549,6 +549,7 @@ pgpa_join_path_setup(PlannerInfo *root, RelOptInfo *joinrel,
                {
                        pgpa_planner_info *proot;
                        MemoryContext oldcontext;
+                       Bitmapset *relids;
 
                        /*
                         * Get or create a pgpa_planner_info object, and then add the
@@ -558,12 +559,20 @@ pgpa_join_path_setup(PlannerInfo *root, RelOptInfo *joinrel,
                         * context, since we might have been called by GEQO. We want all
                         * the data we store here (including the proot, if we create it)
                         * to last for as long as the pgpa_planner_state.
+                        *
+                        * pgpa_filter_out_join_relids copies the input Bitmapset whether
+                        * or not it is changed, so 'relids' is part of the long-lived
+                        * context.
                         */
                        oldcontext = MemoryContextSwitchTo(pps->mcxt);
                        proot = pgpa_planner_get_proot(pps, root);
-                       if (!list_member(proot->sj_unique_rels, uniquerel->relids))
+                       relids = pgpa_filter_out_join_relids(uniquerel->relids,
+                                                                                                root->parse->rtable);
+                       if (!list_member(proot->sj_unique_rels, relids))
                                proot->sj_unique_rels = lappend(proot->sj_unique_rels,
-                                                                                               bms_copy(uniquerel->relids));
+                                                                                               relids);
+                       else
+                               bms_free(relids);
                        MemoryContextSwitchTo(oldcontext);
                }
        }

diff --git a/contrib/pg_plan_advice/sql/semijoin.sql b/contrib/pg_plan_advice/sql/semijoin.sql
index 873f0d3766cefeb207b1c2fb46efa551ac69b430..b4d503f67e3e13e3d2052f0537e8f74a480ef32f 100644 (file)
--- a/contrib/pg_plan_advice/sql/semijoin.sql
+++ b/contrib/pg_plan_advice/sql/semijoin.sql
@@ -125,3 +125,12 @@ SELECT * FROM
        (SELECT * FROM sj_narrow WHERE id IN (SELECT val1 FROM sj_wide)
         LIMIT 1) x,
        LATERAL (SELECT 1 WHERE false) y;
+
+-- Test the case where the planner makes one side of a semijoin unique, and
+-- that side contains an outer join; this test is just to make sure that
+-- advice generation does not fail.
+EXPLAIN (COSTS OFF, PLAN_ADVICE)
+SELECT 1 FROM generate_series(1, 1000) g WHERE EXISTS
+       (SELECT 1 FROM
+               (SELECT 1 FROM (SELECT 1) LEFT JOIN sj_narrow ON true) s,
+               sj_narrow t2 WHERE g = t2.id);