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 998212dda8f167adb49a4f9a050f5e5ab43a359c..d84f66a81b304250c7590c8cf89878fa8c29512e 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2804,7 +2804,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 95476ada0ba69da16e396bacac4f0d08eaa45646..7905bc46548cdafbfee23ef2edc983adb0666d6c 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3898,7 +3898,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 863e0e24a148d74628e87ed169816cbe188098e8..df4ca1291912b7aca7be1fc644fab50aef59cad7 100644 (file)
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -61,7 +61,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;
 
@@ -1157,13 +1156,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
@@ -1226,9 +1221,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 c5ab53e05cb91f9126b66ebd94fb3940106dcd2f..244d1e11974a65e4249b7533daf7a8a2839bf7cd 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -241,7 +241,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 57e145b0f5a3886157a378a1795836797be17d46..32f90d73750a037e7b5e8b24328ebdb6bb85a4ca 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -2535,6 +2535,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 f1ea3ae22e2550287d66baef046d37fc603cb9aa..7e430e859eacb9c26131dbac035880d05b5c5946 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -1181,6 +1181,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