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 37e1d26bda75ee9bc62809e07d4755ae1f2c16a7..f3e7018ed2a06266a01ce2b8582fea2a09a337db 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2461,7 +2461,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 c04b21553d1caae5421d90967caf9d1f6ddf6be4..a2101fb3fcb6c2047a6369602ca319ea3e362348 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3862,7 +3862,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 cb3dde946f52df26d3af732ad971aaf42207b7cd..0ee36b95004e650c7f44b90e8b932a2b9a2d193a 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 8ee40cc68c2b8c433a833e3e67ae677a7cc65bf8..f16466a0df19a7481ac1d7f441f45813d6c7bc9c 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -240,7 +240,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 d17da5c32ad0840eba7f358e7d6cd969f261b6e4..b7f02d6de1b146e2540e32a1d1b94ef1bef1d29d 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -2504,6 +2504,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 2b99e43a0b264ba8a63979cd1cb3df249f1c7ceb..2be7e0866acdf70aca22bd13a037ae259e64d637 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -1173,6 +1173,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