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

Commit2d5b97b

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 parent9e77323 commit2d5b97b

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
@@ -585,7 +585,10 @@ rewriteRuleAction(Query *parsetree,
585585
}
586586
}
587587

588-
/* OK, it's safe to combine the CTE lists */
588+
/*
589+
* OK, it's safe to combine the CTE lists. Beware that RewriteQuery
590+
* knows we concatenate the lists in this order.
591+
*/
589592
sub_action->cteList=list_concat(sub_action->cteList,
590593
copyObject(parsetree->cteList));
591594
/* ... and don't forget about the associated flags */
@@ -3677,9 +3680,13 @@ rewriteTargetView(Query *parsetree, Relation view)
36773680
* orig_rt_length is the length of the originating query's rtable, for product
36783681
* queries created by fireRules(), and 0 otherwise. This is used to skip any
36793682
* already-processed VALUES RTEs from the original query.
3683+
*
3684+
* num_ctes_processed is the number of CTEs at the end of the query's cteList
3685+
* that have already been rewritten, and must not be rewritten again.
36803686
*/
36813687
staticList*
3682-
RewriteQuery(Query*parsetree,List*rewrite_events,intorig_rt_length)
3688+
RewriteQuery(Query*parsetree,List*rewrite_events,intorig_rt_length,
3689+
intnum_ctes_processed)
36833690
{
36843691
CmdTypeevent=parsetree->commandType;
36853692
boolinstead= false;
@@ -3693,17 +3700,29 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
36933700
* First, recursively process any insert/update/delete statements in WITH
36943701
* clauses. (We have to do this first because the WITH clauses may get
36953702
* copied into rule actions below.)
3703+
*
3704+
* Any new WITH clauses from rule actions are processed when we recurse
3705+
* into product queries below. However, when recursing, we must take care
3706+
* to avoid rewriting a CTE query more than once (because expanding
3707+
* generated columns in the targetlist more than once would fail). Since
3708+
* new CTEs from product queries are added to the start of the list (see
3709+
* rewriteRuleAction), we just skip the last num_ctes_processed items.
36963710
*/
36973711
foreach(lc1,parsetree->cteList)
36983712
{
36993713
CommonTableExpr*cte=lfirst_node(CommonTableExpr,lc1);
37003714
Query*ctequery=castNode(Query,cte->ctequery);
3715+
inti=foreach_current_index(lc1);
37013716
List*newstuff;
37023717

3718+
/* Skip already-processed CTEs at the end of the list */
3719+
if (i >=list_length(parsetree->cteList)-num_ctes_processed)
3720+
break;
3721+
37033722
if (ctequery->commandType==CMD_SELECT)
37043723
continue;
37053724

3706-
newstuff=RewriteQuery(ctequery,rewrite_events,0);
3725+
newstuff=RewriteQuery(ctequery,rewrite_events,0,0);
37073726

37083727
/*
37093728
* Currently we can only handle unconditional, single-statement DO
@@ -3762,6 +3781,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
37623781
errmsg("multi-statement DO INSTEAD rules are not supported for data-modifying statements in WITH")));
37633782
}
37643783
}
3784+
num_ctes_processed=list_length(parsetree->cteList);
37653785

37663786
/*
37673787
* If the statement is an insert, update, or delete, adjust its targetlist
@@ -4087,7 +4107,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
40874107
newstuff=RewriteQuery(pt,rewrite_events,
40884108
pt==parsetree ?
40894109
orig_rt_length :
4090-
product_orig_rt_length);
4110+
product_orig_rt_length,
4111+
num_ctes_processed);
40914112
rewritten=list_concat(rewritten,newstuff);
40924113
}
40934114

@@ -4239,7 +4260,7 @@ QueryRewrite(Query *parsetree)
42394260
*
42404261
* Apply all non-SELECT rules possibly getting 0 or many queries
42414262
*/
4242-
querylist=RewriteQuery(parsetree,NIL,0);
4263+
querylist=RewriteQuery(parsetree,NIL,0,0);
42434264

42444265
/*
42454266
* Step 2

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,6 +2767,47 @@ SELECT * FROM bug6051_3;
27672767
---
27682768
(0 rows)
27692769

2770+
-- check that recursive CTE processing doesn't rewrite a CTE more than once
2771+
-- (must not try to expand GENERATED ALWAYS IDENTITY columns more than once)
2772+
CREATE TEMP TABLE id_alw1 (i int GENERATED ALWAYS AS IDENTITY);
2773+
CREATE TEMP TABLE id_alw2 (i int GENERATED ALWAYS AS IDENTITY);
2774+
CREATE TEMP VIEW id_alw2_view AS SELECT * FROM id_alw2;
2775+
CREATE TEMP TABLE id_alw3 (i int GENERATED ALWAYS AS IDENTITY);
2776+
CREATE RULE id_alw3_ins AS ON INSERT TO id_alw3 DO INSTEAD
2777+
WITH t1 AS (INSERT INTO id_alw1 DEFAULT VALUES RETURNING i)
2778+
INSERT INTO id_alw2_view DEFAULT VALUES RETURNING i;
2779+
CREATE TEMP VIEW id_alw3_view AS SELECT * FROM id_alw3;
2780+
CREATE TEMP TABLE id_alw4 (i int GENERATED ALWAYS AS IDENTITY);
2781+
WITH t4 AS (INSERT INTO id_alw4 DEFAULT VALUES RETURNING i)
2782+
INSERT INTO id_alw3_view DEFAULT VALUES RETURNING i;
2783+
i
2784+
---
2785+
1
2786+
(1 row)
2787+
2788+
SELECT * from id_alw1;
2789+
i
2790+
---
2791+
1
2792+
(1 row)
2793+
2794+
SELECT * from id_alw2;
2795+
i
2796+
---
2797+
1
2798+
(1 row)
2799+
2800+
SELECT * from id_alw3;
2801+
i
2802+
---
2803+
(0 rows)
2804+
2805+
SELECT * from id_alw4;
2806+
i
2807+
---
2808+
1
2809+
(1 row)
2810+
27702811
-- check case where CTE reference is removed due to optimization
27712812
EXPLAIN (VERBOSE, COSTS OFF)
27722813
SELECT q1 FROM

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

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

13201320
SELECT*FROM bug6051_3;
13211321

1322+
-- check that recursive CTE processing doesn't rewrite a CTE more than once
1323+
-- (must not try to expand GENERATED ALWAYS IDENTITY columns more than once)
1324+
CREATE TEMP TABLE id_alw1 (iint GENERATED ALWAYSAS IDENTITY);
1325+
1326+
CREATE TEMP TABLE id_alw2 (iint GENERATED ALWAYSAS IDENTITY);
1327+
CREATE TEMP VIEW id_alw2_viewASSELECT*FROM id_alw2;
1328+
1329+
CREATE TEMP TABLE id_alw3 (iint GENERATED ALWAYSAS IDENTITY);
1330+
CREATERULEid_alw3_insASON INSERT TO id_alw3 DO INSTEAD
1331+
WITH t1AS (INSERT INTO id_alw1 DEFAULTVALUES RETURNING i)
1332+
INSERT INTO id_alw2_view DEFAULTVALUES RETURNING i;
1333+
CREATE TEMP VIEW id_alw3_viewASSELECT*FROM id_alw3;
1334+
1335+
CREATE TEMP TABLE id_alw4 (iint GENERATED ALWAYSAS IDENTITY);
1336+
1337+
WITH t4AS (INSERT INTO id_alw4 DEFAULTVALUES RETURNING i)
1338+
INSERT INTO id_alw3_view DEFAULTVALUES RETURNING i;
1339+
1340+
SELECT*from id_alw1;
1341+
SELECT*from id_alw2;
1342+
SELECT*from id_alw3;
1343+
SELECT*from id_alw4;
1344+
13221345
-- check case where CTE reference is removed due to optimization
13231346
EXPLAIN (VERBOSE, COSTS OFF)
13241347
SELECT q1FROM

0 commit comments

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