Skip to content

Commit 92e7a53

Browse files
tglsfdctglsfdc
committed
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
1 parent 914611e commit 92e7a53

File tree

6 files changed

+101
-11
lines changed

6 files changed

+101
-11
lines changed

‎src/backend/optimizer/path/allpaths.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2804,7 +2804,8 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
28042804
if (ndx >= list_length(cteroot->cte_plan_ids))
28052805
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
28062806
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
2807-
Assert(plan_id > 0);
2807+
if (plan_id <= 0)
2808+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
28082809
cteplan = (Plan *) list_nth(root->glob->subplans, plan_id - 1);
28092810

28102811
/* Mark rel with estimated output rows, width, etc */

‎src/backend/optimizer/plan/createplan.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3898,7 +3898,8 @@ create_ctescan_plan(PlannerInfo *root, Path *best_path,
38983898
if (ndx >= list_length(cteroot->cte_plan_ids))
38993899
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
39003900
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
3901-
Assert(plan_id > 0);
3901+
if (plan_id <= 0)
3902+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
39023903
foreach(lc, cteroot->init_plans)
39033904
{
39043905
ctesplan = (SubPlan *) lfirst(lc);

‎src/backend/optimizer/plan/subselect.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ typedef struct inline_cte_walker_context
6161
{
6262
const char *ctename; /* name and relative level of target CTE */
6363
int levelsup;
64-
int refcount; /* number of remaining references */
6564
Query *ctequery; /* query to substitute */
6665
} inline_cte_walker_context;
6766

@@ -1157,13 +1156,9 @@ inline_cte(PlannerInfo *root, CommonTableExpr *cte)
11571156
context.ctename = cte->ctename;
11581157
/* Start at levelsup = -1 because we'll immediately increment it */
11591158
context.levelsup = -1;
1160-
context.refcount = cte->cterefcount;
11611159
context.ctequery = castNode(Query, cte->ctequery);
11621160

11631161
(void) inline_cte_walker((Node *) root->parse, &context);
1164-
1165-
/* Assert we replaced all references */
1166-
Assert(context.refcount == 0);
11671162
}
11681163

11691164
static bool
@@ -1226,9 +1221,6 @@ inline_cte_walker(Node *node, inline_cte_walker_context *context)
12261221
rte->coltypes = NIL;
12271222
rte->coltypmods = NIL;
12281223
rte->colcollations = NIL;
1229-
1230-
/* Count the number of replacements we've done */
1231-
context->refcount--;
12321224
}
12331225

12341226
return false;

‎src/include/nodes/pathnodes.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,8 @@ struct PlannerInfo
241241

242242
List *init_plans; /* init SubPlans for query */
243243

244-
List *cte_plan_ids; /* per-CTE-item list of subplan IDs */
244+
List *cte_plan_ids; /* per-CTE-item list of subplan IDs (or -1 if
245+
* no subplan was made for that CTE) */
245246

246247
List *multiexpr_params; /* List of Lists of Params for MULTIEXPR
247248
* subquery outputs */

‎src/test/regress/expected/with.out

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2535,6 +2535,70 @@ SELECT * FROM bug6051_3;
25352535
---
25362536
(0 rows)
25372537

2538+
-- check case where CTE reference is removed due to optimization
2539+
EXPLAIN (VERBOSE, COSTS OFF)
2540+
SELECT q1 FROM
2541+
(
2542+
WITH t_cte AS (SELECT * FROM int8_tbl t)
2543+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2544+
FROM int8_tbl i8
2545+
) ss;
2546+
QUERY PLAN
2547+
--------------------------------------
2548+
Subquery Scan on ss
2549+
Output: ss.q1
2550+
-> Seq Scan on public.int8_tbl i8
2551+
Output: i8.q1, NULL::bigint
2552+
(4 rows)
2553+
2554+
SELECT q1 FROM
2555+
(
2556+
WITH t_cte AS (SELECT * FROM int8_tbl t)
2557+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2558+
FROM int8_tbl i8
2559+
) ss;
2560+
q1
2561+
------------------
2562+
123
2563+
123
2564+
4567890123456789
2565+
4567890123456789
2566+
4567890123456789
2567+
(5 rows)
2568+
2569+
EXPLAIN (VERBOSE, COSTS OFF)
2570+
SELECT q1 FROM
2571+
(
2572+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
2573+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2574+
FROM int8_tbl i8
2575+
) ss;
2576+
QUERY PLAN
2577+
---------------------------------------------
2578+
Subquery Scan on ss
2579+
Output: ss.q1
2580+
-> Seq Scan on public.int8_tbl i8
2581+
Output: i8.q1, NULL::bigint
2582+
CTE t_cte
2583+
-> Seq Scan on public.int8_tbl t
2584+
Output: t.q1, t.q2
2585+
(7 rows)
2586+
2587+
SELECT q1 FROM
2588+
(
2589+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
2590+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2591+
FROM int8_tbl i8
2592+
) ss;
2593+
q1
2594+
------------------
2595+
123
2596+
123
2597+
4567890123456789
2598+
4567890123456789
2599+
4567890123456789
2600+
(5 rows)
2601+
25382602
-- a truly recursive CTE in the same list
25392603
WITH RECURSIVE t(a) AS (
25402604
SELECT 0

‎src/test/regress/sql/with.sql

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,37 @@ COMMIT;
11811181

11821182
SELECT * FROM bug6051_3;
11831183

1184+
-- check case where CTE reference is removed due to optimization
1185+
EXPLAIN (VERBOSE, COSTS OFF)
1186+
SELECT q1 FROM
1187+
(
1188+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1189+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1190+
FROM int8_tbl i8
1191+
) ss;
1192+
1193+
SELECT q1 FROM
1194+
(
1195+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1196+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1197+
FROM int8_tbl i8
1198+
) ss;
1199+
1200+
EXPLAIN (VERBOSE, COSTS OFF)
1201+
SELECT q1 FROM
1202+
(
1203+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
1204+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1205+
FROM int8_tbl i8
1206+
) ss;
1207+
1208+
SELECT q1 FROM
1209+
(
1210+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
1211+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1212+
FROM int8_tbl i8
1213+
) ss;
1214+
11841215
-- a truly recursive CTE in the same list
11851216
WITH RECURSIVE t(a) AS (
11861217
SELECT 0

0 commit comments

Comments
 (0)