Re: Assert failure in CTE inlining with view and correlated subquery

Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> On 4/21/22 10:29, Richard Guo wrote:
>> Further debugging shows that in this repro the reference to the CTE is
>> removed when generating paths for the subquery 'sub', where we would try
>> to remove subquery targetlist items that are not needed. So for the
>> items we are to remove, maybe we need to check if they contain CTEs and
>> if so decrease cterefcount of the CTEs correspondingly.

> Right, at some point we remove the unnecessary targetlist entries, but
> that ignores the entry may reference a CTE. That's pretty much what I
> meant by the counter being "out of sync".
> Updating the counter while removing the entry is one option, but maybe
> we could simply delay counting the CTE references until after that?

I think we should just drop this cross-check altogether; it is not nearly
useful enough to justify the work that'd be involved in maintaining
cterefcount accurately for all such transformations.  All it's really
there for is to be sure that we don't need to make a subplan for the
inlined CTE.

There are two downstream consumers of cte_plan_ids, which currently just
have Asserts that we made a subplan.  I think it'd be worth converting
those to real run-time tests, which leads me to something more or less as
attached.

            regards, tom lane

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 998212dda8..d84f66a81b 100644
--- 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 95476ada0b..7905bc4654 100644
--- 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 863e0e24a1..df4ca12919 100644
--- 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 c5ab53e05c..244d1e1197 100644
--- 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 */