pg_plan_advice: Fix a bug when a subquery is pruned away entirely.

If a subquery is proven empty, and if that subquery contained a
semijoin, and if making one side or the other of that semijoin
unique and performing an inner join was a possible strategy, then
the previous code would fail with ERROR: no rtoffset for plan %s
when attempting to generate advice. Fix that.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: http://postgr.es/m/CA+TgmobOOmmXSJz3e+cjTY-bA1+W0dqVDqzxUBEvGtW62whYGg@mail.gmail.com

diff --git a/contrib/pg_plan_advice/expected/semijoin.out b/contrib/pg_plan_advice/expected/semijoin.out
index 5551c028a1fd18d9ad216231f8cdd51ea98e0be4..680de215117e616ca22848ac5a51e35b28cc0de8 100644 (file)
--- a/contrib/pg_plan_advice/expected/semijoin.out
+++ b/contrib/pg_plan_advice/expected/semijoin.out
@@ -375,3 +375,20 @@ SELECT * FROM generate_series(1,1000) g, sj_narrow s WHERE g = s.val1;
 (13 rows)
 
 COMMIT;
+-- Test the case where the subquery containing a semijoin is removed from
+-- the query entirely; this test is just to make sure that advice generation
+-- does not fail.
+EXPLAIN (COSTS OFF, PLAN_ADVICE)
+SELECT * FROM
+       (SELECT * FROM sj_narrow WHERE id IN (SELECT val1 FROM sj_wide)
+        LIMIT 1) x,
+       LATERAL (SELECT 1 WHERE false) y;
+        QUERY PLAN        
+--------------------------
+ Result
+   Replaces: Scan on x
+   One-Time Filter: false
+ Generated Plan Advice:
+   NO_GATHER(x)
+(5 rows)
+

diff --git a/contrib/pg_plan_advice/pgpa_planner.c b/contrib/pg_plan_advice/pgpa_planner.c
index b25f62b2e87c88d23a24bdce401e46ace0ab4183..469a08424277c2e803b291e3ea2a26e2040bbe3f 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_planner.c
+++ b/contrib/pg_plan_advice/pgpa_planner.c
@@ -2066,6 +2066,9 @@ pgpa_compute_rt_identifier(pgpa_planner_info *proot, PlannerInfo *root,
 /*
  * Compute the range table offset for each pgpa_planner_info for which it
  * is possible to meaningfully do so.
+ *
+ * For pgpa_planner_info objects for which no RT offset can be computed,
+ * clear sj_unique_rels, which is meaningless in such cases.
  */
 static void
 pgpa_compute_rt_offsets(pgpa_planner_state *pps, PlannedStmt *pstmt)
@@ -2097,23 +2100,24 @@ pgpa_compute_rt_offsets(pgpa_planner_state *pps, PlannedStmt *pstmt)
                                 * there's no fixed rtoffset that we can apply to the RTIs
                                 * used during planning to locate the corresponding relations.
                                 */
-                               if (rtinfo->dummy)
+                               if (!rtinfo->dummy)
                                {
-                                       /*
-                                        * It will not be possible to make any effective use of
-                                        * the sj_unique_rels list in this case, and it also won't
-                                        * be important to do so. So just throw the list away to
-                                        * avoid confusing pgpa_plan_walker.
-                                        */
-                                       proot->sj_unique_rels = NIL;
-                                       break;
+                                       Assert(!proot->has_rtoffset);
+                                       proot->has_rtoffset = true;
+                                       proot->rtoffset = rtinfo->rtoffset;
                                }
-                               Assert(!proot->has_rtoffset);
-                               proot->has_rtoffset = true;
-                               proot->rtoffset = rtinfo->rtoffset;
                                break;
                        }
                }
+
+               /*
+                * If we didn't end up setting has_rtoffset, then it will not be
+                * possible to make any effective use of sj_unique_rels, and it also
+                * won't be important to do so.  So just throw the list away to avoid
+                * confusing pgpa_plan_walker.
+                */
+               if (!proot->has_rtoffset)
+                       proot->sj_unique_rels = NIL;
        }
 }
 

diff --git a/contrib/pg_plan_advice/sql/semijoin.sql b/contrib/pg_plan_advice/sql/semijoin.sql
index 5a4ae52d1d9b68a631d418641e4630b59fff955c..873f0d3766cefeb207b1c2fb46efa551ac69b430 100644 (file)
--- a/contrib/pg_plan_advice/sql/semijoin.sql
+++ b/contrib/pg_plan_advice/sql/semijoin.sql
@@ -116,3 +116,12 @@ SET LOCAL pg_plan_advice.advice = 'semijoin_unique(g)';
 EXPLAIN (COSTS OFF, PLAN_ADVICE)
 SELECT * FROM generate_series(1,1000) g, sj_narrow s WHERE g = s.val1;
 COMMIT;
+
+-- Test the case where the subquery containing a semijoin is removed from
+-- the query entirely; this test is just to make sure that advice generation
+-- does not fail.
+EXPLAIN (COSTS OFF, PLAN_ADVICE)
+SELECT * FROM
+       (SELECT * FROM sj_narrow WHERE id IN (SELECT val1 FROM sj_wide)
+        LIMIT 1) x,
+       LATERAL (SELECT 1 WHERE false) y;