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

Commitad77039

Browse files
committed
Calculate extraUpdatedCols in query rewriter, not parser.
It's unsafe to do this at parse time because addition of generatedcolumns to a table would not invalidate stored rules containingUPDATEs on the table ... but there might now be dependent generatedcolumns that were not there when the rule was made. This also fixesan oversight that rewriteTargetView failed to update extraUpdatedColswhen transforming an UPDATE on an updatable view. (Since the newcalculation is downstream of that, rewriteTargetView doesn't actuallyneed to do anything; but before, there was a demonstrable bug there.)In v13 and HEAD, this leads to easily-visible bugs because (sincecommitc6679e4) we won't recalculate generated columns that aren'tlisted in extraUpdatedCols. In v12 this bitmap is mostly just usedfor trigger-firing decisions, so you'd only notice a problem if atrigger cared whether a generated column had been updated.I'd complained about this back in May, but then forgot about ituntil bug #16671 from Michael Paul Killian revived the issue.Back-patch to v12 where this field was introduced. If existingstored rules contain any extraUpdatedCols values, they'll beignored because the rewriter will overwrite them, so the bug willbe fixed even for existing rules. (But note that if someone wereto update to 13.1 or 12.5, store some rules with UPDATEs on tableshaving generated columns, and then downgrade to a prior minor version,they might observe issues similar to what this patch fixes. Thatseems unlikely enough to not be worth going to a lot of effort to fix.)Discussion:https://postgr.es/m/10206.1588964727@sss.pgh.pa.usDiscussion:https://postgr.es/m/16671-2fa55851859fb166@postgresql.org
1 parent36b9312 commitad77039

File tree

9 files changed

+115
-47
lines changed

9 files changed

+115
-47
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,9 @@ flatten_rtes_walker(Node *node, PlannerGlobal *glob)
438438
* In the flat rangetable, we zero out substructure pointers that are not
439439
* needed by the executor; this reduces the storage space and copying cost
440440
* for cached plans. We keep only the ctename, alias and eref Alias fields,
441-
* which are needed by EXPLAIN, and the selectedCols, insertedCols and
442-
* updatedColsbitmaps, which are needed for executor-startup permissions
443-
* checking and for trigger event checking.
441+
* which are needed by EXPLAIN, and the selectedCols, insertedCols,
442+
* updatedCols, and extraUpdatedColsbitmaps, which are needed for
443+
*executor-startup permissionschecking and for trigger event checking.
444444
*/
445445
staticvoid
446446
add_rte_to_flat_rtable(PlannerGlobal*glob,RangeTblEntry*rte)

‎src/backend/parser/analyze.c

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2282,7 +2282,6 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
22822282
RangeTblEntry*target_rte;
22832283
ListCell*orig_tl;
22842284
ListCell*tl;
2285-
TupleDesctupdesc=pstate->p_target_relation->rd_att;
22862285

22872286
tlist=transformTargetList(pstate,origTlist,
22882287
EXPR_KIND_UPDATE_SOURCE);
@@ -2341,41 +2340,9 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
23412340
if (orig_tl!=NULL)
23422341
elog(ERROR,"UPDATE target count mismatch --- internal error");
23432342

2344-
fill_extraUpdatedCols(target_rte,tupdesc);
2345-
23462343
returntlist;
23472344
}
23482345

2349-
/*
2350-
* Record in extraUpdatedCols generated columns referencing updated base
2351-
* columns.
2352-
*/
2353-
void
2354-
fill_extraUpdatedCols(RangeTblEntry*target_rte,TupleDesctupdesc)
2355-
{
2356-
if (tupdesc->constr&&
2357-
tupdesc->constr->has_generated_stored)
2358-
{
2359-
for (inti=0;i<tupdesc->constr->num_defval;i++)
2360-
{
2361-
AttrDefaultdefval=tupdesc->constr->defval[i];
2362-
Node*expr;
2363-
Bitmapset*attrs_used=NULL;
2364-
2365-
/* skip if not generated column */
2366-
if (!TupleDescAttr(tupdesc,defval.adnum-1)->attgenerated)
2367-
continue;
2368-
2369-
expr=stringToNode(defval.adbin);
2370-
pull_varattnos(expr,1,&attrs_used);
2371-
2372-
if (bms_overlap(target_rte->updatedCols,attrs_used))
2373-
target_rte->extraUpdatedCols=bms_add_member(target_rte->extraUpdatedCols,
2374-
defval.adnum-FirstLowInvalidHeapAttributeNumber);
2375-
}
2376-
}
2377-
}
2378-
23792346
/*
23802347
* transformReturningList -
23812348
*handle a RETURNING clause in INSERT/UPDATE/DELETE

‎src/backend/replication/logical/worker.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@
8181
#include"miscadmin.h"
8282
#include"nodes/makefuncs.h"
8383
#include"optimizer/optimizer.h"
84-
#include"parser/analyze.h"
85-
#include"parser/parse_relation.h"
8684
#include"pgstat.h"
8785
#include"postmaster/bgworker.h"
8886
#include"postmaster/interrupt.h"
@@ -1323,7 +1321,8 @@ apply_handle_update(StringInfo s)
13231321
}
13241322
}
13251323

1326-
fill_extraUpdatedCols(target_rte,RelationGetDescr(rel->localrel));
1324+
/* Also populate extraUpdatedCols, in case we have generated columns */
1325+
fill_extraUpdatedCols(target_rte,rel->localrel);
13271326

13281327
PushActiveSnapshot(GetTransactionSnapshot());
13291328

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include"miscadmin.h"
3131
#include"nodes/makefuncs.h"
3232
#include"nodes/nodeFuncs.h"
33+
#include"optimizer/optimizer.h"
3334
#include"parser/analyze.h"
3435
#include"parser/parse_coerce.h"
3536
#include"parser/parse_relation.h"
@@ -1508,6 +1509,42 @@ rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
15081509
}
15091510
}
15101511

1512+
/*
1513+
* Record in target_rte->extraUpdatedCols the indexes of any generated columns
1514+
* that depend on any columns mentioned in target_rte->updatedCols.
1515+
*/
1516+
void
1517+
fill_extraUpdatedCols(RangeTblEntry*target_rte,Relationtarget_relation)
1518+
{
1519+
TupleDesctupdesc=RelationGetDescr(target_relation);
1520+
TupleConstr*constr=tupdesc->constr;
1521+
1522+
target_rte->extraUpdatedCols=NULL;
1523+
1524+
if (constr&&constr->has_generated_stored)
1525+
{
1526+
for (inti=0;i<constr->num_defval;i++)
1527+
{
1528+
AttrDefault*defval=&constr->defval[i];
1529+
Node*expr;
1530+
Bitmapset*attrs_used=NULL;
1531+
1532+
/* skip if not generated column */
1533+
if (!TupleDescAttr(tupdesc,defval->adnum-1)->attgenerated)
1534+
continue;
1535+
1536+
/* identify columns this generated column depends on */
1537+
expr=stringToNode(defval->adbin);
1538+
pull_varattnos(expr,1,&attrs_used);
1539+
1540+
if (bms_overlap(target_rte->updatedCols,attrs_used))
1541+
target_rte->extraUpdatedCols=
1542+
bms_add_member(target_rte->extraUpdatedCols,
1543+
defval->adnum-FirstLowInvalidHeapAttributeNumber);
1544+
}
1545+
}
1546+
}
1547+
15111548

15121549
/*
15131550
* matchLocks -
@@ -1639,6 +1676,7 @@ ApplyRetrieveRule(Query *parsetree,
16391676
rte->selectedCols=NULL;
16401677
rte->insertedCols=NULL;
16411678
rte->updatedCols=NULL;
1679+
rte->extraUpdatedCols=NULL;
16421680

16431681
/*
16441682
* For the most part, Vars referencing the view should remain as
@@ -3617,6 +3655,9 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
36173655
parsetree->override,
36183656
rt_entry_relation,
36193657
parsetree->resultRelation);
3658+
3659+
/* Also populate extraUpdatedCols (for generated columns) */
3660+
fill_extraUpdatedCols(rt_entry,rt_entry_relation);
36203661
}
36213662
elseif (event==CMD_DELETE)
36223663
{

‎src/include/nodes/parsenodes.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -940,12 +940,16 @@ typedef struct PartitionCmd
940940
*
941941
* updatedCols is also used in some other places, for example, to determine
942942
* which triggers to fire and in FDWs to know which changed columns they
943-
* need to ship off. Generated columns that are caused to be updated by an
944-
* update to a base column are collected in extraUpdatedCols. This is not
945-
* considered for permission checking, but it is useful in those places
946-
* that want to know the full set of columns being updated as opposed to
947-
* only the ones the user explicitly mentioned in the query. (There is
948-
* currently no need for an extraInsertedCols, but it could exist.)
943+
* need to ship off.
944+
*
945+
* Generated columns that are caused to be updated by an update to a base
946+
* column are listed in extraUpdatedCols. This is not considered for
947+
* permission checking, but it is useful in those places that want to know
948+
* the full set of columns being updated as opposed to only the ones the
949+
* user explicitly mentioned in the query. (There is currently no need for
950+
* an extraInsertedCols, but it could exist.) Note that extraUpdatedCols
951+
* is populated during query rewrite, NOT in the parser, since generated
952+
* columns could be added after a rule has been parsed and stored.
949953
*
950954
* securityQuals is a list of security barrier quals (boolean expressions),
951955
* to be tested in the listed order before returning a row from the

‎src/include/parser/analyze.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,4 @@ extern void applyLockingClause(Query *qry, Index rtindex,
4646
externList*BuildOnConflictExcludedTargetlist(Relationtargetrel,
4747
IndexexclRelIndex);
4848

49-
externvoidfill_extraUpdatedCols(RangeTblEntry*target_rte,TupleDesctupdesc);
50-
5149
#endif/* ANALYZE_H */

‎src/include/rewrite/rewriteHandler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ extern Node *build_column_default(Relation rel, int attrno);
2626
externvoidrewriteTargetListUD(Query*parsetree,RangeTblEntry*target_rte,
2727
Relationtarget_relation);
2828

29+
externvoidfill_extraUpdatedCols(RangeTblEntry*target_rte,
30+
Relationtarget_relation);
31+
2932
externQuery*get_view_query(Relationview);
3033
externconstchar*view_query_is_auto_updatable(Query*viewquery,
3134
boolcheck_cols);

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,41 @@ NOTICE: drop cascades to 3 other objects
14671467
DETAIL: drop cascades to view rw_view1
14681468
drop cascades to view rw_view2
14691469
drop cascades to view rw_view3
1470+
-- view on table with GENERATED columns
1471+
CREATE TABLE base_tbl (id int, idplus1 int GENERATED ALWAYS AS (id + 1) STORED);
1472+
CREATE VIEW rw_view1 AS SELECT * FROM base_tbl;
1473+
INSERT INTO base_tbl (id) VALUES (1);
1474+
INSERT INTO rw_view1 (id) VALUES (2);
1475+
INSERT INTO base_tbl (id, idplus1) VALUES (3, DEFAULT);
1476+
INSERT INTO rw_view1 (id, idplus1) VALUES (4, DEFAULT);
1477+
INSERT INTO base_tbl (id, idplus1) VALUES (5, 6); -- error
1478+
ERROR: cannot insert into column "idplus1"
1479+
DETAIL: Column "idplus1" is a generated column.
1480+
INSERT INTO rw_view1 (id, idplus1) VALUES (6, 7); -- error
1481+
ERROR: cannot insert into column "idplus1"
1482+
DETAIL: Column "idplus1" is a generated column.
1483+
SELECT * FROM base_tbl;
1484+
id | idplus1
1485+
----+---------
1486+
1 | 2
1487+
2 | 3
1488+
3 | 4
1489+
4 | 5
1490+
(4 rows)
1491+
1492+
UPDATE base_tbl SET id = 2000 WHERE id = 2;
1493+
UPDATE rw_view1 SET id = 3000 WHERE id = 3;
1494+
SELECT * FROM base_tbl;
1495+
id | idplus1
1496+
------+---------
1497+
1 | 2
1498+
4 | 5
1499+
2000 | 2001
1500+
3000 | 3001
1501+
(4 rows)
1502+
1503+
DROP TABLE base_tbl CASCADE;
1504+
NOTICE: drop cascades to view rw_view1
14701505
-- inheritance tests
14711506
CREATE TABLE base_tbl_parent (a int);
14721507
CREATE TABLE base_tbl_child (CHECK (a > 0)) INHERITS (base_tbl_parent);

‎src/test/regress/sql/updatable_views.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,27 @@ SELECT events & 4 != 0 AS upd,
697697

698698
DROPTABLE base_tbl CASCADE;
699699

700+
-- view on table with GENERATED columns
701+
702+
CREATETABLEbase_tbl (idint, idplus1int GENERATED ALWAYSAS (id+1) STORED);
703+
CREATEVIEWrw_view1ASSELECT*FROM base_tbl;
704+
705+
INSERT INTO base_tbl (id)VALUES (1);
706+
INSERT INTO rw_view1 (id)VALUES (2);
707+
INSERT INTO base_tbl (id, idplus1)VALUES (3, DEFAULT);
708+
INSERT INTO rw_view1 (id, idplus1)VALUES (4, DEFAULT);
709+
INSERT INTO base_tbl (id, idplus1)VALUES (5,6);-- error
710+
INSERT INTO rw_view1 (id, idplus1)VALUES (6,7);-- error
711+
712+
SELECT*FROM base_tbl;
713+
714+
UPDATE base_tblSET id=2000WHERE id=2;
715+
UPDATE rw_view1SET id=3000WHERE id=3;
716+
717+
SELECT*FROM base_tbl;
718+
719+
DROPTABLE base_tbl CASCADE;
720+
700721
-- inheritance tests
701722

702723
CREATETABLEbase_tbl_parent (aint);

0 commit comments

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