Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3881561

Browse files
committed
Avoid rewriting data-modifying CTEs more than once.
Formerly, when updating an auto-updatable view, or a relation withrules, if the original query had any data-modifying CTEs, the rewriterwould rewrite those CTEs multiple times as RewriteQuery() recursedinto the product queries. In most cases that was harmless, becauseRewriteQuery() is mostly idempotent. However, if the CTE involvedupdating an always-generated column, it would trigger an error becauseany subsequent rewrite would appear to be attempting to assign anon-default value to the always-generated column.This could perhaps be fixed by attempting to make RewriteQuery() fullyidempotent, but that looks quite tricky to achieve, and would probablybe quite fragile, given that more generated-column-type features mightbe added in the future.Instead, fix by arranging for RewriteQuery() to rewrite each CTEexactly once (by tracking the number of CTEs already rewritten as itrecurses). This has the advantage of being simpler and more efficient,but it does make RewriteQuery() dependent on the order in whichrewriteRuleAction() joins the CTE lists from the original query andthe rule action, so care must be taken if that is ever changed.Reported-by: Bernice Southey <bernice.southey@gmail.com>Author: Bernice Southey <bernice.southey@gmail.com>Author: Dean Rasheed <dean.a.rasheed@gmail.com>Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>Discussion:https://postgr.es/m/CAEDh4nyD6MSH9bROhsOsuTqGAv_QceU_GDvN9WcHLtZTCYM1kA@mail.gmail.comBackpatch-through: 14
1 parent87c6f8b commit3881561

File tree

3 files changed

+90
-5
lines changed

3 files changed

+90
-5
lines changed

‎src/backend/rewrite/rewriteHandler.c‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,10 @@ rewriteRuleAction(Query *parsetree,
592592
}
593593
}
594594

595-
/* OK, it's safe to combine the CTE lists */
595+
/*
596+
* OK, it's safe to combine the CTE lists. Beware that RewriteQuery
597+
* knows we concatenate the lists in this order.
598+
*/
596599
sub_action->cteList=list_concat(sub_action->cteList,
597600
copyObject(parsetree->cteList));
598601
/* ... and don't forget about the associated flags */
@@ -3872,9 +3875,13 @@ rewriteTargetView(Query *parsetree, Relation view)
38723875
* orig_rt_length is the length of the originating query's rtable, for product
38733876
* queries created by fireRules(), and 0 otherwise. This is used to skip any
38743877
* already-processed VALUES RTEs from the original query.
3878+
*
3879+
* num_ctes_processed is the number of CTEs at the end of the query's cteList
3880+
* that have already been rewritten, and must not be rewritten again.
38753881
*/
38763882
staticList*
3877-
RewriteQuery(Query*parsetree,List*rewrite_events,intorig_rt_length)
3883+
RewriteQuery(Query*parsetree,List*rewrite_events,intorig_rt_length,
3884+
intnum_ctes_processed)
38783885
{
38793886
CmdTypeevent=parsetree->commandType;
38803887
boolinstead= false;
@@ -3888,17 +3895,29 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
38883895
* First, recursively process any insert/update/delete/merge statements in
38893896
* WITH clauses. (We have to do this first because the WITH clauses may
38903897
* get copied into rule actions below.)
3898+
*
3899+
* Any new WITH clauses from rule actions are processed when we recurse
3900+
* into product queries below. However, when recursing, we must take care
3901+
* to avoid rewriting a CTE query more than once (because expanding
3902+
* generated columns in the targetlist more than once would fail). Since
3903+
* new CTEs from product queries are added to the start of the list (see
3904+
* rewriteRuleAction), we just skip the last num_ctes_processed items.
38913905
*/
38923906
foreach(lc1,parsetree->cteList)
38933907
{
38943908
CommonTableExpr*cte=lfirst_node(CommonTableExpr,lc1);
38953909
Query*ctequery=castNode(Query,cte->ctequery);
3910+
inti=foreach_current_index(lc1);
38963911
List*newstuff;
38973912

3913+
/* Skip already-processed CTEs at the end of the list */
3914+
if (i >=list_length(parsetree->cteList)-num_ctes_processed)
3915+
break;
3916+
38983917
if (ctequery->commandType==CMD_SELECT)
38993918
continue;
39003919

3901-
newstuff=RewriteQuery(ctequery,rewrite_events,0);
3920+
newstuff=RewriteQuery(ctequery,rewrite_events,0,0);
39023921

39033922
/*
39043923
* Currently we can only handle unconditional, single-statement DO
@@ -3958,6 +3977,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
39583977
errmsg("multi-statement DO INSTEAD rules are not supported for data-modifying statements in WITH")));
39593978
}
39603979
}
3980+
num_ctes_processed=list_length(parsetree->cteList);
39613981

39623982
/*
39633983
* If the statement is an insert, update, delete, or merge, adjust its
@@ -4289,7 +4309,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
42894309
newstuff=RewriteQuery(pt,rewrite_events,
42904310
pt==parsetree ?
42914311
orig_rt_length :
4292-
product_orig_rt_length);
4312+
product_orig_rt_length,
4313+
num_ctes_processed);
42934314
rewritten=list_concat(rewritten,newstuff);
42944315
}
42954316

@@ -4564,7 +4585,7 @@ QueryRewrite(Query *parsetree)
45644585
*
45654586
* Apply all non-SELECT rules possibly getting 0 or many queries
45664587
*/
4567-
querylist=RewriteQuery(parsetree,NIL,0);
4588+
querylist=RewriteQuery(parsetree,NIL,0,0);
45684589

45694590
/*
45704591
* Step 2

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2872,6 +2872,47 @@ SELECT * FROM bug6051_3;
28722872
---
28732873
(0 rows)
28742874

2875+
-- check that recursive CTE processing doesn't rewrite a CTE more than once
2876+
-- (must not try to expand GENERATED ALWAYS IDENTITY columns more than once)
2877+
CREATE TEMP TABLE id_alw1 (i int GENERATED ALWAYS AS IDENTITY);
2878+
CREATE TEMP TABLE id_alw2 (i int GENERATED ALWAYS AS IDENTITY);
2879+
CREATE TEMP VIEW id_alw2_view AS SELECT * FROM id_alw2;
2880+
CREATE TEMP TABLE id_alw3 (i int GENERATED ALWAYS AS IDENTITY);
2881+
CREATE RULE id_alw3_ins AS ON INSERT TO id_alw3 DO INSTEAD
2882+
WITH t1 AS (INSERT INTO id_alw1 DEFAULT VALUES RETURNING i)
2883+
INSERT INTO id_alw2_view DEFAULT VALUES RETURNING i;
2884+
CREATE TEMP VIEW id_alw3_view AS SELECT * FROM id_alw3;
2885+
CREATE TEMP TABLE id_alw4 (i int GENERATED ALWAYS AS IDENTITY);
2886+
WITH t4 AS (INSERT INTO id_alw4 DEFAULT VALUES RETURNING i)
2887+
INSERT INTO id_alw3_view DEFAULT VALUES RETURNING i;
2888+
i
2889+
---
2890+
1
2891+
(1 row)
2892+
2893+
SELECT * from id_alw1;
2894+
i
2895+
---
2896+
1
2897+
(1 row)
2898+
2899+
SELECT * from id_alw2;
2900+
i
2901+
---
2902+
1
2903+
(1 row)
2904+
2905+
SELECT * from id_alw3;
2906+
i
2907+
---
2908+
(0 rows)
2909+
2910+
SELECT * from id_alw4;
2911+
i
2912+
---
2913+
1
2914+
(1 row)
2915+
28752916
-- check case where CTE reference is removed due to optimization
28762917
EXPLAIN (VERBOSE, COSTS OFF)
28772918
SELECT q1 FROM

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,29 @@ COMMIT;
13601360

13611361
SELECT*FROM bug6051_3;
13621362

1363+
-- check that recursive CTE processing doesn't rewrite a CTE more than once
1364+
-- (must not try to expand GENERATED ALWAYS IDENTITY columns more than once)
1365+
CREATE TEMP TABLE id_alw1 (iint GENERATED ALWAYSAS IDENTITY);
1366+
1367+
CREATE TEMP TABLE id_alw2 (iint GENERATED ALWAYSAS IDENTITY);
1368+
CREATE TEMP VIEW id_alw2_viewASSELECT*FROM id_alw2;
1369+
1370+
CREATE TEMP TABLE id_alw3 (iint GENERATED ALWAYSAS IDENTITY);
1371+
CREATERULEid_alw3_insASON INSERT TO id_alw3 DO INSTEAD
1372+
WITH t1AS (INSERT INTO id_alw1 DEFAULTVALUES RETURNING i)
1373+
INSERT INTO id_alw2_view DEFAULTVALUES RETURNING i;
1374+
CREATE TEMP VIEW id_alw3_viewASSELECT*FROM id_alw3;
1375+
1376+
CREATE TEMP TABLE id_alw4 (iint GENERATED ALWAYSAS IDENTITY);
1377+
1378+
WITH t4AS (INSERT INTO id_alw4 DEFAULTVALUES RETURNING i)
1379+
INSERT INTO id_alw3_view DEFAULTVALUES RETURNING i;
1380+
1381+
SELECT*from id_alw1;
1382+
SELECT*from id_alw2;
1383+
SELECT*from id_alw3;
1384+
SELECT*from id_alw4;
1385+
13631386
-- check case where CTE reference is removed due to optimization
13641387
EXPLAIN (VERBOSE, COSTS OFF)
13651388
SELECT q1FROM

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp