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

Commit43330cd

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 parent93f726c commit43330cd

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
@@ -387,9 +387,9 @@ flatten_rtes_walker(Node *node, PlannerGlobal *glob)
387387
* In the flat rangetable, we zero out substructure pointers that are not
388388
* needed by the executor; this reduces the storage space and copying cost
389389
* for cached plans. We keep only the ctename, alias and eref Alias fields,
390-
* which are needed by EXPLAIN, and the selectedCols, insertedCols and
391-
* updatedColsbitmaps, which are needed for executor-startup permissions
392-
* checking and for trigger event checking.
390+
* which are needed by EXPLAIN, and the selectedCols, insertedCols,
391+
* updatedCols, and extraUpdatedColsbitmaps, which are needed for
392+
*executor-startup permissionschecking and for trigger event checking.
393393
*/
394394
staticvoid
395395
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
@@ -2287,7 +2287,6 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
22872287
RangeTblEntry*target_rte;
22882288
ListCell*orig_tl;
22892289
ListCell*tl;
2290-
TupleDesctupdesc=pstate->p_target_relation->rd_att;
22912290

22922291
tlist=transformTargetList(pstate,origTlist,
22932292
EXPR_KIND_UPDATE_SOURCE);
@@ -2346,41 +2345,9 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
23462345
if (orig_tl!=NULL)
23472346
elog(ERROR,"UPDATE target count mismatch --- internal error");
23482347

2349-
fill_extraUpdatedCols(target_rte,tupdesc);
2350-
23512348
returntlist;
23522349
}
23532350

2354-
/*
2355-
* Record in extraUpdatedCols generated columns referencing updated base
2356-
* columns.
2357-
*/
2358-
void
2359-
fill_extraUpdatedCols(RangeTblEntry*target_rte,TupleDesctupdesc)
2360-
{
2361-
if (tupdesc->constr&&
2362-
tupdesc->constr->has_generated_stored)
2363-
{
2364-
for (inti=0;i<tupdesc->constr->num_defval;i++)
2365-
{
2366-
AttrDefaultdefval=tupdesc->constr->defval[i];
2367-
Node*expr;
2368-
Bitmapset*attrs_used=NULL;
2369-
2370-
/* skip if not generated column */
2371-
if (!TupleDescAttr(tupdesc,defval.adnum-1)->attgenerated)
2372-
continue;
2373-
2374-
expr=stringToNode(defval.adbin);
2375-
pull_varattnos(expr,1,&attrs_used);
2376-
2377-
if (bms_overlap(target_rte->updatedCols,attrs_used))
2378-
target_rte->extraUpdatedCols=bms_add_member(target_rte->extraUpdatedCols,
2379-
defval.adnum-FirstLowInvalidHeapAttributeNumber);
2380-
}
2381-
}
2382-
}
2383-
23842351
/*
23852352
* transformReturningList -
23862353
*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
@@ -42,8 +42,6 @@
4242
#include"miscadmin.h"
4343
#include"nodes/makefuncs.h"
4444
#include"optimizer/optimizer.h"
45-
#include"parser/analyze.h"
46-
#include"parser/parse_relation.h"
4745
#include"pgstat.h"
4846
#include"postmaster/bgworker.h"
4947
#include"postmaster/postmaster.h"
@@ -744,7 +742,8 @@ apply_handle_update(StringInfo s)
744742
}
745743
}
746744

747-
fill_extraUpdatedCols(target_rte,RelationGetDescr(rel->localrel));
745+
/* Also populate extraUpdatedCols, in case we have generated columns */
746+
fill_extraUpdatedCols(target_rte,rel->localrel);
748747

749748
PushActiveSnapshot(GetTransactionSnapshot());
750749
ExecOpenIndices(estate->es_result_relation_info, false);

‎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"
@@ -1510,6 +1511,42 @@ rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
15101511
}
15111512
}
15121513

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

15141551
/*
15151552
* matchLocks -
@@ -1641,6 +1678,7 @@ ApplyRetrieveRule(Query *parsetree,
16411678
rte->selectedCols=NULL;
16421679
rte->insertedCols=NULL;
16431680
rte->updatedCols=NULL;
1681+
rte->extraUpdatedCols=NULL;
16441682

16451683
/*
16461684
* 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
@@ -937,12 +937,16 @@ typedef struct PartitionCmd
937937
*
938938
* updatedCols is also used in some other places, for example, to determine
939939
* which triggers to fire and in FDWs to know which changed columns they
940-
* need to ship off. Generated columns that are caused to be updated by an
941-
* update to a base column are collected in extraUpdatedCols. This is not
942-
* considered for permission checking, but it is useful in those places
943-
* that want to know the full set of columns being updated as opposed to
944-
* only the ones the user explicitly mentioned in the query. (There is
945-
* currently no need for an extraInsertedCols, but it could exist.)
940+
* need to ship off.
941+
*
942+
* Generated columns that are caused to be updated by an update to a base
943+
* column are listed in extraUpdatedCols. This is not considered for
944+
* permission checking, but it is useful in those places that want to know
945+
* the full set of columns being updated as opposed to only the ones the
946+
* user explicitly mentioned in the query. (There is currently no need for
947+
* an extraInsertedCols, but it could exist.) Note that extraUpdatedCols
948+
* is populated during query rewrite, NOT in the parser, since generated
949+
* columns could be added after a rule has been parsed and stored.
946950
*
947951
* securityQuals is a list of security barrier quals (boolean expressions),
948952
* 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