Skip to content

Commit da22ef3

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 b235d41 commit da22ef3

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
@@ -2461,7 +2461,8 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
24612461
if (ndx >= list_length(cteroot->cte_plan_ids))
24622462
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
24632463
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
2464-
Assert(plan_id > 0);
2464+
if (plan_id <= 0)
2465+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
24652466
cteplan = (Plan *) list_nth(root->glob->subplans, plan_id - 1);
24662467

24672468
/* 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
@@ -3862,7 +3862,8 @@ create_ctescan_plan(PlannerInfo *root, Path *best_path,
38623862
if (ndx >= list_length(cteroot->cte_plan_ids))
38633863
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
38643864
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
3865-
Assert(plan_id > 0);
3865+
if (plan_id <= 0)
3866+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
38663867
foreach(lc, cteroot->init_plans)
38673868
{
38683869
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
@@ -240,7 +240,8 @@ struct PlannerInfo
240240

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

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

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

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2504,6 +2504,70 @@ SELECT * FROM bug6051_3;
25042504
---
25052505
(0 rows)
25062506

2507+
-- check case where CTE reference is removed due to optimization
2508+
EXPLAIN (VERBOSE, COSTS OFF)
2509+
SELECT q1 FROM
2510+
(
2511+
WITH t_cte AS (SELECT * FROM int8_tbl t)
2512+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2513+
FROM int8_tbl i8
2514+
) ss;
2515+
QUERY PLAN
2516+
--------------------------------------
2517+
Subquery Scan on ss
2518+
Output: ss.q1
2519+
-> Seq Scan on public.int8_tbl i8
2520+
Output: i8.q1, NULL::bigint
2521+
(4 rows)
2522+
2523+
SELECT q1 FROM
2524+
(
2525+
WITH t_cte AS (SELECT * FROM int8_tbl t)
2526+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2527+
FROM int8_tbl i8
2528+
) ss;
2529+
q1
2530+
------------------
2531+
123
2532+
123
2533+
4567890123456789
2534+
4567890123456789
2535+
4567890123456789
2536+
(5 rows)
2537+
2538+
EXPLAIN (VERBOSE, COSTS OFF)
2539+
SELECT q1 FROM
2540+
(
2541+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
2542+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2543+
FROM int8_tbl i8
2544+
) ss;
2545+
QUERY PLAN
2546+
---------------------------------------------
2547+
Subquery Scan on ss
2548+
Output: ss.q1
2549+
-> Seq Scan on public.int8_tbl i8
2550+
Output: i8.q1, NULL::bigint
2551+
CTE t_cte
2552+
-> Seq Scan on public.int8_tbl t
2553+
Output: t.q1, t.q2
2554+
(7 rows)
2555+
2556+
SELECT q1 FROM
2557+
(
2558+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
2559+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
2560+
FROM int8_tbl i8
2561+
) ss;
2562+
q1
2563+
------------------
2564+
123
2565+
123
2566+
4567890123456789
2567+
4567890123456789
2568+
4567890123456789
2569+
(5 rows)
2570+
25072571
-- a truly recursive CTE in the same list
25082572
WITH RECURSIVE t(a) AS (
25092573
SELECT 0

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

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

11741174
SELECT * FROM bug6051_3;
11751175

1176+
-- check case where CTE reference is removed due to optimization
1177+
EXPLAIN (VERBOSE, COSTS OFF)
1178+
SELECT q1 FROM
1179+
(
1180+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1181+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1182+
FROM int8_tbl i8
1183+
) ss;
1184+
1185+
SELECT q1 FROM
1186+
(
1187+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1188+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1189+
FROM int8_tbl i8
1190+
) ss;
1191+
1192+
EXPLAIN (VERBOSE, COSTS OFF)
1193+
SELECT q1 FROM
1194+
(
1195+
WITH t_cte AS MATERIALIZED (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+
SELECT q1 FROM
1201+
(
1202+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
1203+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1204+
FROM int8_tbl i8
1205+
) ss;
1206+
11761207
-- a truly recursive CTE in the same list
11771208
WITH RECURSIVE t(a) AS (
11781209
SELECT 0

0 commit comments

Comments
 (0)