Remove inadequate assertion check in CTE inlining.

inline_cte() expected to find exactly as many references to the
target CTE as its cterefcount indicates.  While that should be
accurate for the tree as emitted by the parser, there are some
optimizations that occur upstream of here that could falsify it,
notably removal of unused subquery output expressions.

Trying to make the accounting 100% accurate seems expensive and
doomed to future breakage.  It's not really worth it, because
all this code is protecting is downstream assumptions that every
referenced CTE has a plan.  Let's convert those assertions to
regular test-and-elog just in case there's some actual problem,
and then drop the failing assertion.

Per report from Tomas Vondra (thanks also to Richard Guo for
analysis).  Back-patch to v12 where the faulty code came in.

Discussion: https://postgr.es/m/29196a1e-ed47-c7ca-9be2-b1c636816183@enterprisedb.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 8a518d445bf89c53180acacc1ad1407244e90161..439337df958dabe9b92a0b2676c30aa64e51c239 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2541,7 +2541,8 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
        if (ndx >= list_length(cteroot->cte_plan_ids))
                elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
        plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
-       Assert(plan_id > 0);
+       if (plan_id <= 0)
+               elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
        cteplan = (Plan *) list_nth(root->glob->subplans, plan_id - 1);
 
        /* Mark rel with estimated output rows, width, etc */

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 1fe1ff734905472f8f25d5cdfa29338939da8ee9..01c042171354bb8b25260e98ff64d1f98e6d3979 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3636,7 +3636,8 @@ create_ctescan_plan(PlannerInfo *root, Path *best_path,
        if (ndx >= list_length(cteroot->cte_plan_ids))
                elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
        plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
-       Assert(plan_id > 0);
+       if (plan_id <= 0)
+               elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
        foreach(lc, cteroot->init_plans)
        {
                ctesplan = (SubPlan *) lfirst(lc);

diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 5eb40a05f8b52864d4ea07b6ff5fb4e6fe1f5241..528e2a362440f357d0142ec930e939d18272b809 100644 (file)
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -64,7 +64,6 @@ typedef struct inline_cte_walker_context
 {
        const char *ctename;            /* name and relative level of target CTE */
        int                     levelsup;
-       int                     refcount;               /* number of remaining references */
        Query      *ctequery;           /* query to substitute */
 } inline_cte_walker_context;
 
@@ -1131,13 +1130,9 @@ inline_cte(PlannerInfo *root, CommonTableExpr *cte)
        context.ctename = cte->ctename;
        /* Start at levelsup = -1 because we'll immediately increment it */
        context.levelsup = -1;
-       context.refcount = cte->cterefcount;
        context.ctequery = castNode(Query, cte->ctequery);
 
        (void) inline_cte_walker((Node *) root->parse, &context);
-
-       /* Assert we replaced all references */
-       Assert(context.refcount == 0);
 }
 
 static bool
@@ -1200,9 +1195,6 @@ inline_cte_walker(Node *node, inline_cte_walker_context *context)
                        rte->coltypes = NIL;
                        rte->coltypmods = NIL;
                        rte->colcollations = NIL;
-
-                       /* Count the number of replacements we've done */
-                       context->refcount--;
                }
 
                return false;

diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index b6d4b0eda0ab25d14c369df45f17c77180e0ac50..2e8663473894c71c9db8e1efad307ce4569edf98 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -258,7 +258,8 @@ struct PlannerInfo
 
        List       *init_plans;         /* init SubPlans for query */
 
-       List       *cte_plan_ids;       /* per-CTE-item list of subplan IDs */
+       List       *cte_plan_ids;       /* per-CTE-item list of subplan IDs (or -1 if
+                                                                * no subplan was made for that CTE) */
 
        List       *multiexpr_params;   /* List of Lists of Params for MULTIEXPR
                                                                         * subquery outputs */

diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 10ef7c7cce13a3488b1d5c8fdb13d5d1dd5f2487..98c8314c86f7789ed627b4837cc10b3134a0b48b 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1707,6 +1707,70 @@ SELECT * FROM bug6051_3;
 ---
 (0 rows)
 
+-- check case where CTE reference is removed due to optimization
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT q1 FROM
+(
+  WITH t_cte AS (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+              QUERY PLAN              
+--------------------------------------
+ Subquery Scan on ss
+   Output: ss.q1
+   ->  Seq Scan on public.int8_tbl i8
+         Output: i8.q1, NULL::bigint
+(4 rows)
+
+SELECT q1 FROM
+(
+  WITH t_cte AS (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+        q1        
+------------------
+              123
+              123
+ 4567890123456789
+ 4567890123456789
+ 4567890123456789
+(5 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT q1 FROM
+(
+  WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+                 QUERY PLAN                  
+---------------------------------------------
+ Subquery Scan on ss
+   Output: ss.q1
+   ->  Seq Scan on public.int8_tbl i8
+         Output: i8.q1, NULL::bigint
+         CTE t_cte
+           ->  Seq Scan on public.int8_tbl t
+                 Output: t.q1, t.q2
+(7 rows)
+
+SELECT q1 FROM
+(
+  WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+        q1        
+------------------
+              123
+              123
+ 4567890123456789
+ 4567890123456789
+ 4567890123456789
+(5 rows)
+
 -- a truly recursive CTE in the same list
 WITH RECURSIVE t(a) AS (
        SELECT 0

diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index b9242b7913c4abfd554bb44e47076e33ba2f339d..4243903581d271cdc1e9cb4648fff98053704461 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -798,6 +798,37 @@ COMMIT;
 
 SELECT * FROM bug6051_3;
 
+-- check case where CTE reference is removed due to optimization
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT q1 FROM
+(
+  WITH t_cte AS (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+
+SELECT q1 FROM
+(
+  WITH t_cte AS (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT q1 FROM
+(
+  WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+
+SELECT q1 FROM
+(
+  WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
+  SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
+  FROM int8_tbl i8
+) ss;
+
 -- a truly recursive CTE in the same list
 WITH RECURSIVE t(a) AS (
        SELECT 0