Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e6440d5

Browse files
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 1c60a99 commit e6440d5

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
@@ -2538,7 +2538,8 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
25382538
if (ndx >= list_length(cteroot->cte_plan_ids))
25392539
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
25402540
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
2541-
Assert(plan_id > 0);
2541+
if (plan_id <= 0)
2542+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
25422543
cteplan = (Plan *) list_nth(root->glob->subplans, plan_id - 1);
25432544

25442545
/* 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
@@ -3709,7 +3709,8 @@ create_ctescan_plan(PlannerInfo *root, Path *best_path,
37093709
if (ndx >= list_length(cteroot->cte_plan_ids))
37103710
elog(ERROR, "could not find plan for CTE \"%s\"", rte->ctename);
37113711
plan_id = list_nth_int(cteroot->cte_plan_ids, ndx);
3712-
Assert(plan_id > 0);
3712+
if (plan_id <= 0)
3713+
elog(ERROR, "no plan was made for CTE \"%s\"", rte->ctename);
37133714
foreach(lc, cteroot->init_plans)
37143715
{
37153716
ctesplan = (SubPlan *) lfirst(lc);

src/backend/optimizer/plan/subselect.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ typedef struct inline_cte_walker_context
6464
{
6565
const char *ctename; /* name and relative level of target CTE */
6666
int levelsup;
67-
int refcount; /* number of remaining references */
6867
Query *ctequery; /* query to substitute */
6968
} inline_cte_walker_context;
7069

@@ -1131,13 +1130,9 @@ inline_cte(PlannerInfo *root, CommonTableExpr *cte)
11311130
context.ctename = cte->ctename;
11321131
/* Start at levelsup = -1 because we'll immediately increment it */
11331132
context.levelsup = -1;
1134-
context.refcount = cte->cterefcount;
11351133
context.ctequery = castNode(Query, cte->ctequery);
11361134

11371135
(void) inline_cte_walker((Node *) root->parse, &context);
1138-
1139-
/* Assert we replaced all references */
1140-
Assert(context.refcount == 0);
11411136
}
11421137

11431138
static bool
@@ -1200,9 +1195,6 @@ inline_cte_walker(Node *node, inline_cte_walker_context *context)
12001195
rte->coltypes = NIL;
12011196
rte->coltypmods = NIL;
12021197
rte->colcollations = NIL;
1203-
1204-
/* Count the number of replacements we've done */
1205-
context->refcount--;
12061198
}
12071199

12081200
return false;

src/include/nodes/pathnodes.h

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

259259
List *init_plans; /* init SubPlans for query */
260260

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

263264
List *multiexpr_params; /* List of Lists of Params for MULTIEXPR
264265
* subquery outputs */

src/test/regress/expected/with.out

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,6 +1766,70 @@ SELECT * FROM bug6051_3;
17661766
---
17671767
(0 rows)
17681768

1769+
-- check case where CTE reference is removed due to optimization
1770+
EXPLAIN (VERBOSE, COSTS OFF)
1771+
SELECT q1 FROM
1772+
(
1773+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1774+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1775+
FROM int8_tbl i8
1776+
) ss;
1777+
QUERY PLAN
1778+
--------------------------------------
1779+
Subquery Scan on ss
1780+
Output: ss.q1
1781+
-> Seq Scan on public.int8_tbl i8
1782+
Output: i8.q1, NULL::bigint
1783+
(4 rows)
1784+
1785+
SELECT q1 FROM
1786+
(
1787+
WITH t_cte AS (SELECT * FROM int8_tbl t)
1788+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1789+
FROM int8_tbl i8
1790+
) ss;
1791+
q1
1792+
------------------
1793+
123
1794+
123
1795+
4567890123456789
1796+
4567890123456789
1797+
4567890123456789
1798+
(5 rows)
1799+
1800+
EXPLAIN (VERBOSE, COSTS OFF)
1801+
SELECT q1 FROM
1802+
(
1803+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
1804+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1805+
FROM int8_tbl i8
1806+
) ss;
1807+
QUERY PLAN
1808+
---------------------------------------------
1809+
Subquery Scan on ss
1810+
Output: ss.q1
1811+
-> Seq Scan on public.int8_tbl i8
1812+
Output: i8.q1, NULL::bigint
1813+
CTE t_cte
1814+
-> Seq Scan on public.int8_tbl t
1815+
Output: t.q1, t.q2
1816+
(7 rows)
1817+
1818+
SELECT q1 FROM
1819+
(
1820+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
1821+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
1822+
FROM int8_tbl i8
1823+
) ss;
1824+
q1
1825+
------------------
1826+
123
1827+
123
1828+
4567890123456789
1829+
4567890123456789
1830+
4567890123456789
1831+
(5 rows)
1832+
17691833
-- a truly recursive CTE in the same list
17701834
WITH RECURSIVE t(a) AS (
17711835
SELECT 0

src/test/regress/sql/with.sql

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

843843
SELECT * FROM bug6051_3;
844844

845+
-- check case where CTE reference is removed due to optimization
846+
EXPLAIN (VERBOSE, COSTS OFF)
847+
SELECT q1 FROM
848+
(
849+
WITH t_cte AS (SELECT * FROM int8_tbl t)
850+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
851+
FROM int8_tbl i8
852+
) ss;
853+
854+
SELECT q1 FROM
855+
(
856+
WITH t_cte AS (SELECT * FROM int8_tbl t)
857+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
858+
FROM int8_tbl i8
859+
) ss;
860+
861+
EXPLAIN (VERBOSE, COSTS OFF)
862+
SELECT q1 FROM
863+
(
864+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
865+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
866+
FROM int8_tbl i8
867+
) ss;
868+
869+
SELECT q1 FROM
870+
(
871+
WITH t_cte AS MATERIALIZED (SELECT * FROM int8_tbl t)
872+
SELECT q1, (SELECT q2 FROM t_cte WHERE t_cte.q1 = i8.q1) AS t_sub
873+
FROM int8_tbl i8
874+
) ss;
875+
845876
-- a truly recursive CTE in the same list
846877
WITH RECURSIVE t(a) AS (
847878
SELECT 0

0 commit comments

Comments
 (0)