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 e7a3e92bc2090d30d1d4134a54ecd18dcd51d34f..3723863a85d84ce5f622106fcd472a7aa16902f0 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2538,7 +2538,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 e445debe5716f6764555fd1a656bb7b8292e8742..917713c163397d6e018ff3a1bd07a85f222306b8 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3709,7 +3709,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 11e29dd1536de3ab271b0efe6294381096546132..91a8851b25310cf4cd2d9e3a2445227b2dc279b6 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 69150e46ebdc4175a59d64b2bef21f94e936d3c6..5ebf0709799af2decdf18b920eef001412020423 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 04a942993d6faf89a57e43f2d9c729b00f8e2ca6..63bbef0677e5b2a45df25a7125a3388ecc256162 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1766,6 +1766,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 75b310cac5d1d3f20845251a916ff0a3c8ef9911..d2c4c174a45c09ee35e127648034a4d441e3386a 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -842,6 +842,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